From 165f5045fb135da95ee6ff5b5388f44a1dd6a918 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Wed, 18 Feb 2015 14:24:16 -0800 Subject: [PATCH] feat(quoted-text): Edit quoted text in replies Summary: Use blockquote, apply gmail's styling Fix in popout composer so the ... button rests at the bottom Test Plan: Need to figure out the best strategy for tests here - will write tomorrow Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1197 --- exports/inbox-exports.coffee | 3 + .../lib/contenteditable-component.cjsx | 79 +++++++++--- .../spec/contenteditable-component-spec.cjsx | 121 ++++++++++++++++++ .../composer/stylesheets/composer.less | 57 ++++----- .../message-list/lib/message-item.cjsx | 19 +-- src/components/quoted-text-toggle-button.cjsx | 34 +++++ src/flux/models/utils.coffee | 9 ++ src/flux/stores/draft-store.coffee | 13 +- static/atom.less | 1 + static/components/extra.less | 30 +++++ 10 files changed, 301 insertions(+), 65 deletions(-) create mode 100644 internal_packages/composer/spec/contenteditable-component-spec.cjsx create mode 100644 src/components/quoted-text-toggle-button.cjsx create mode 100644 static/components/extra.less diff --git a/exports/inbox-exports.coffee b/exports/inbox-exports.coffee index 065d39fd4..8ec31dffb 100644 --- a/exports/inbox-exports.coffee +++ b/exports/inbox-exports.coffee @@ -40,3 +40,6 @@ module.exports = # Component Registry ComponentRegistry: require '../src/component-registry' + + # Utils + Utils: require '../src/flux/models/utils' \ No newline at end of file diff --git a/internal_packages/composer/lib/contenteditable-component.cjsx b/internal_packages/composer/lib/contenteditable-component.cjsx index 0e9a3cb32..f31c460cc 100644 --- a/internal_packages/composer/lib/contenteditable-component.cjsx +++ b/internal_packages/composer/lib/contenteditable-component.cjsx @@ -1,38 +1,66 @@ _ = require 'underscore-plus' React = require 'react' sanitizeHtml = require 'sanitize-html' +{Utils} = require 'inbox-exports' module.exports = ContenteditableComponent = React.createClass - render: -> -
- shouldComponentUpdate: (nextProps) -> - html = @getDOMNode().innerHTML - return (nextProps.html isnt html) and (document.activeElement isnt @getDOMNode()) + getInitialState: -> + editQuotedText: false + + getEditableNode: -> + @refs.contenteditable.getDOMNode() + + render: -> + quotedTextClass = React.addons.classSet + "quoted-text-toggle": true + 'hidden': @_htmlQuotedTextStart() is -1 + 'state-on': @state.editQuotedText + +
+
+ +
+ + shouldComponentUpdate: (nextProps, nextState) -> + return true if nextState.editQuotedText is not @state.editQuotedText + + html = @getEditableNode().innerHTML + return (nextProps.html isnt html) and (document.activeElement isnt @getEditableNode()) componentDidUpdate: -> - if ( @props.html != @getDOMNode().innerHTML ) - @getDOMNode().innerHTML = @props.html + if (@props.html != @getEditableNode().innerHTML) + @getEditableNode().innerHTML = @_htmlForDisplay() focus: -> - @getDOMNode().focus() + @getEditableNode().focus() _onChange: (evt) -> - html = @getDOMNode().innerHTML - if (@props.onChange && html != @lastHtml) - evt.target = { value: html } - @props.onChange(evt) + html = @getEditableNode().innerHTML + + # If we aren't displaying quoted text, add the quoted + # text to the end of the visible text + if not @state.editQuotedText + quoteStart = @_htmlQuotedTextStart() + html += @props.html.substr(quoteStart) + + if html != @lastHtml + @props.onChange({target: {value: html}}) if @props.onChange @lastHtml = html + _onToggleQuotedText: -> + @setState + editQuotedText: !@state.editQuotedText + _onPaste: (evt) -> html = evt.clipboardData.getData("text/html") ? "" if html.length is 0 @@ -68,3 +96,14 @@ ContenteditableComponent = React.createClass table: "p" cleanHtml.replace(/

/gi, "").replace(/<\/p>/gi, "

") + + _htmlQuotedTextStart: -> + @props.html.search(/<[^>]*gmail_quote/) + + _htmlForDisplay: -> + if @state.editQuotedText + @props.html + else + quoteStart = @_htmlQuotedTextStart() + @props.html.substr(0, quoteStart) unless quoteStart is -1 + diff --git a/internal_packages/composer/spec/contenteditable-component-spec.cjsx b/internal_packages/composer/spec/contenteditable-component-spec.cjsx new file mode 100644 index 000000000..4be8427ec --- /dev/null +++ b/internal_packages/composer/spec/contenteditable-component-spec.cjsx @@ -0,0 +1,121 @@ +_ = require "underscore-plus" +React = require "react/addons" + +ReactTestUtils = React.addons.TestUtils +ReactTestUtils.scryRenderedDOMComponentsWithAttr = (root, attrName, attrValue) -> + ReactTestUtils.findAllInRenderedTree root, (inst) -> + inst.props[attrName] and (!attrValue or inst.props[attrName] is attrValue) + +ReactTestUtils.findRenderedDOMComponentWithAttr = (root, attrName, attrValue) -> + all = ReactTestUtils.scryRenderedDOMComponentsWithAttr(root, attrName, attrValue) + if all.length is not 1 + throw new Error("Did not find exactly one match for data attribute: #{attrName} with value: #{attrValue}") + all[0] + +ContenteditableComponent = require "../lib/contenteditable-component.cjsx", + +describe "ContenteditableComponent", -> + beforeEach -> + @onChange = jasmine.createSpy('onChange') + html = 'Test HTML' + @component = ReactTestUtils.renderIntoDocument( + + ) + + html = 'Test HTML

QUOTE
' + @componentWithQuote = ReactTestUtils.renderIntoDocument( + + ) + + describe "render", -> + it 'should render into the document', -> + expect(ReactTestUtils.isCompositeComponentWithType @component, ContenteditableComponent).toBe true + + it "should include a content-editable div", -> + expect(ReactTestUtils.findRenderedDOMComponentWithAttr(@component, 'contentEditable')).toBeDefined() + + describe "quoted-text-toggle", -> + it "should be rendered", -> + expect(ReactTestUtils.findRenderedDOMComponentWithClass(@component, 'quoted-text-toggle')).toBeDefined() + + it "should be visible if the html contains quoted text", -> + @toggle = ReactTestUtils.findRenderedDOMComponentWithClass(@componentWithQuote, 'quoted-text-toggle') + expect(@toggle.props.className.indexOf('hidden') >= 0).toBe(false) + + it "should be have `state-on` if editQuotedText is true", -> + @componentWithQuote.setState(editQuotedText: true) + @toggle = ReactTestUtils.findRenderedDOMComponentWithClass(@componentWithQuote, 'quoted-text-toggle') + expect(@toggle.props.className.indexOf('state-on') >= 0).toBe(true) + + it "should not have `state-on` if editQuotedText is false", -> + @componentWithQuote.setState(editQuotedText: false) + @toggle = ReactTestUtils.findRenderedDOMComponentWithClass(@componentWithQuote, 'quoted-text-toggle') + expect(@toggle.props.className.indexOf('state-on') >= 0).toBe(false) + + it "should be hidden otherwise", -> + @toggle = ReactTestUtils.findRenderedDOMComponentWithClass(@component, 'quoted-text-toggle') + expect(@toggle.props.className.indexOf('hidden') >= 0).toBe(true) + + describe "when editQuotedText is false", -> + it "should only display HTML up to the beginning of the quoted text", -> + @editDiv = ReactTestUtils.findRenderedDOMComponentWithAttr(@componentWithQuote, 'contentEditable') + expect(@editDiv.getDOMNode().innerHTML.indexOf('gmail_quote') >= 0).toBe(false) + + describe "when editQuotedText is true", -> + it "should display all the HTML", -> + @componentWithQuote.setState(editQuotedText: true) + @editDiv = ReactTestUtils.findRenderedDOMComponentWithAttr(@componentWithQuote, 'contentEditable') + expect(@editDiv.getDOMNode().innerHTML.indexOf('gmail_quote') >= 0).toBe(true) + + describe "editQuotedText", -> + it "should default to false", -> + expect(@component.state.editQuotedText).toBe(false) + + describe "when the html is changed", -> + beforeEach -> + @changedHtmlWithoutQuote = 'Changed NEW 1 HTML
' + @changedHtmlWithQuote = 'Changed NEW 1 HTML
QUOTE
' + + @performEdit = (newHTML, component = @componentWithQuote) => + editDiv = ReactTestUtils.findRenderedDOMComponentWithAttr(component, 'contentEditable') + editDiv.getDOMNode().innerHTML = newHTML + ReactTestUtils.Simulate.input(editDiv, {target: {value: newHTML}}) + + it "should fire `props.onChange`", -> + @performEdit('Test New HTML') + expect(@onChange).toHaveBeenCalled() + + it "should not fire if the html is the same", -> + expect(@onChange.callCount).toBe(0) + @performEdit(@changedHtmlWithoutQuote) + expect(@onChange.callCount).toBe(1) + @performEdit(@changedHtmlWithoutQuote) + expect(@onChange.callCount).toBe(1) + + describe "when editQuotedText is true", -> + it "should call `props.onChange` with the entire HTML string", -> + @componentWithQuote.setState(editQuotedText: true) + @performEdit(@changedHtmlWithQuote) + ev = @onChange.mostRecentCall.args[0] + expect(ev.target.value).toEqual(@changedHtmlWithQuote) + + it "should allow the quoted text to be changed", -> + changed = 'Test NEW 1 HTML
QUOTE CHANGED!!!
' + @componentWithQuote.setState(editQuotedText: true) + @performEdit(changed) + ev = @onChange.mostRecentCall.args[0] + expect(ev.target.value).toEqual(changed) + + describe "when editQuotedText is false", -> + it "should call `props.onChange` with the entire HTML string, even though the div being edited only contains some of it", -> + @componentWithQuote.setState(editQuotedText: false) + @performEdit(@changedHtmlWithoutQuote) + ev = @onChange.mostRecentCall.args[0] + expect(ev.target.value).toEqual(@changedHtmlWithQuote) + + it "should work if the component does not contain quoted text", -> + changed = 'Hallooo! NEW 1 HTML HTML HTML
' + @component.setState(editQuotedText: true) + @performEdit(changed, @component) + ev = @onChange.mostRecentCall.args[0] + expect(ev.target.value).toEqual(changed) diff --git a/internal_packages/composer/stylesheets/composer.less b/internal_packages/composer/stylesheets/composer.less index b613314af..52fbc1be6 100644 --- a/internal_packages/composer/stylesheets/composer.less +++ b/internal_packages/composer/stylesheets/composer.less @@ -13,6 +13,24 @@ width: 100%; height: 100%; } + .composer-inner-wrap { + .compose-body { + margin-bottom: 0; + position: relative; + + .contenteditable-container { + position:absolute; + width:100%; + height:100%; + + div[contenteditable] { + // Ensure that the contentEditable always fills the window, + // but leaves room for the quoted text marker at the bottom. + min-height: calc(~"100% - 30px"); + } + } + } + } } .composer-inner-wrap { @@ -99,7 +117,15 @@ cursor: text; overflow: auto; margin: 0.7em 15px 15px 15px; - min-height: 150px; + position: relative; + + .quoted-text-toggle { + margin:0; + } + + div[contenteditable] { + min-height: 150px; + } } .compose-field { @@ -160,35 +186,6 @@ } } -.quoted-text-toggle { - background-color: #f7f7f7; - border-radius: 5px; - padding: 7px; - margin-bottom: 10px; - margin-left: 15px; - display: inline-block; - padding-top: 0; - padding-bottom: 2px; - color: #333; - border: 1px solid #eee; - line-height: 16px; - - &.hidden { - display:none; - } - &:hover { - border:1px solid @border-color; - background-color: @background-color-secondary; - text-decoration:none; - } - &.state-true:before { - content:'Hide Quoted Text'; - } - &.state-false:before { - content:'\2022\2022\2022'; - } -} - #new-compose-button { order: 1; diff --git a/internal_packages/message-list/lib/message-item.cjsx b/internal_packages/message-list/lib/message-item.cjsx index 237427c9d..793bc3e9f 100644 --- a/internal_packages/message-list/lib/message-item.cjsx +++ b/internal_packages/message-list/lib/message-item.cjsx @@ -3,7 +3,7 @@ React = require 'react' _ = require 'underscore-plus' EmailFrame = require './email-frame' MessageParticipants = require "./message-participants.cjsx" -{ComponentRegistry, FileDownloadStore} = require 'inbox-exports' +{ComponentRegistry, FileDownloadStore, Utils} = require 'inbox-exports' Autolinker = require 'autolinker' module.exports = @@ -31,9 +31,10 @@ MessageItem = React.createClass @_storeUnlisten() if @_storeUnlisten render: -> - quotedTextClass = "quoted-text-toggle" - quotedTextClass += " hidden" unless @_containsQuotedText() - quotedTextClass += " state-" + @state.showQuotedText + quotedTextClass = React.addons.classSet + "quoted-text-toggle": true + 'hidden': !Utils.containsQuotedText(@props.message.body) + 'state-on': @state.showQuotedText messageIndicators = ComponentRegistry.findAllViewsByRole('MessageIndicator') attachments = @_attachmentComponents() @@ -89,16 +90,6 @@ MessageItem = React.createClass body - _containsQuotedText: -> - # I know this is gross - one day we'll replace it with a nice system. - body = @props.message.body - return false unless body - - regexs = [/
|>)/, /[\n]?[ ]*[>|>]/i, /.gmail_quote/] - for regex in regexs - return true if body.match(regex) - return false - _toggleQuotedText: -> @setState showQuotedText: !@state.showQuotedText diff --git a/src/components/quoted-text-toggle-button.cjsx b/src/components/quoted-text-toggle-button.cjsx new file mode 100644 index 000000000..2a36831dc --- /dev/null +++ b/src/components/quoted-text-toggle-button.cjsx @@ -0,0 +1,34 @@ +React = require 'react/addons' +_ = require 'underscore-plus' +{CompositeDisposable} = require 'event-kit' + +### +### + +module.exports = +QuotedTextToggleButton = React.createClass + render: -> + style = + 'backgroundColor': '#f7f7f7' + 'borderRadius': 5 + 'padding': 7 + 'display': 'inline-block' + 'paddingTop': 0 + 'paddingBottom': '2' + 'color': '#333' + 'border': '1px solid #eee' + 'lineHeight': '16px' + 'marginBottom': 10 + 'marginLeft': 15 + 'cursor': 'pointer' + + if @props.hidden + style.display = 'none' + + if @props.toggled + content = 'Hide Quoted Text' + else + content = "•••" + + {content} + diff --git a/src/flux/models/utils.coffee b/src/flux/models/utils.coffee index 5829c8bbe..bf7e6dc06 100644 --- a/src/flux/models/utils.coffee +++ b/src/flux/models/utils.coffee @@ -75,4 +75,13 @@ utils = tableNameForJoin: (primaryKlass, secondaryKlass) -> "#{primaryKlass.name}-#{secondaryKlass.name}" + containsQuotedText: (html) -> + # I know this is gross - one day we'll replace it with a nice system. + return false unless html + + regexs = [/
|>)/, /[\n]?[ ]*[>|>]/i, /.gmail_quote/] + for regex in regexs + return true if html.match(regex) + return false + module.exports = utils diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee index f96a8ad4b..d23370b57 100644 --- a/src/flux/stores/draft-store.coffee +++ b/src/flux/stores/draft-store.coffee @@ -56,6 +56,7 @@ DraftStore = Reflux.createStore @_findLastMessageFromThread(threadId) .then ({lastMessage, thread}) => @_createNewDraftFromThread thread, + quoted_text: lastMessage.body to: lastMessage.from _onComposeReplyAll: (threadId) -> @@ -64,6 +65,7 @@ DraftStore = Reflux.createStore cc = [].concat(lastMessage.cc, lastMessage.to).filter (p) -> !_.contains([].concat(lastMessage.from, [NamespaceStore.current().me()]), p) @_createNewDraftFromThread thread, + quoted_text: lastMessage.body to: lastMessage.from cc: cc @@ -72,7 +74,7 @@ DraftStore = Reflux.createStore .then ({lastMessage, thread}) => @_createNewDraftFromThread thread, subject: "Fwd: " + thread.subject - body: lastMessage.body + quoted_text: lastMessage.body _findLastMessageFromThread: (threadId) -> new Promise (resolve, reject) -> @@ -88,6 +90,15 @@ DraftStore = Reflux.createStore .catch (args...) -> reject(args...) _createNewDraftFromThread: (thread, attributes={}) -> + if attributes.quoted_text + attributes.body = """ +

+
+ #{attributes.quoted_text} +
""" + delete attributes.quoted_text + draft = new Message _.extend {}, attributes, from: [NamespaceStore.current().me()] date: (new Date) diff --git a/static/atom.less b/static/atom.less index 97ace593c..d431f3e34 100644 --- a/static/atom.less +++ b/static/atom.less @@ -9,3 +9,4 @@ @import "./components/popover"; @import "./components/menu"; @import "./components/tokenizing-text-field"; +@import "./components/extra"; \ No newline at end of file diff --git a/static/components/extra.less b/static/components/extra.less new file mode 100644 index 000000000..5a2ee6e79 --- /dev/null +++ b/static/components/extra.less @@ -0,0 +1,30 @@ + +.quoted-text-toggle { + background-color: #f7f7f7; + border-radius: 5px; + padding: 7px; + display: inline-block; + padding-top: 0; + padding-bottom: 2px; + color: #333; + border: 1px solid #eee; + line-height: 16px; + margin-bottom: 10px; + margin-left: 15px; + cursor: pointer; + + &.hidden { + display:none; + } + &:hover { + border:1px solid @border-color; + background-color: @background-color-secondary; + text-decoration:none; + } + &.state-on:before { + content:'Hide Quoted Text'; + } + &:before { + content:'\2022\2022\2022'; + } +}