From b479e099c1c9af711be7c4d81fd8540ba3862c2d Mon Sep 17 00:00:00 2001 From: Evan Morikawa Date: Mon, 9 Mar 2015 11:17:22 -0700 Subject: [PATCH] feat(messages): expandable message headers Summary: feat(messages): expandable message headers Test Plan: edgehill --test Reviewers: bengotow Reviewed By: bengotow Differential Revision: https://review.inboxapp.com/D1267 --- .../message-list/lib/message-item.cjsx | 47 ++++---- .../message-list/lib/message-list.cjsx | 6 - .../lib/message-participants.cjsx | 63 +++++----- .../message-list/lib/message-timestamp.cjsx | 41 ++++--- .../message-list/spec/message-item-spec.cjsx | 33 ++++-- .../spec/message-participants-spec.cjsx | 109 ++++++++++++------ .../spec/message-timestamp-spec.cjsx | 10 +- .../stylesheets/message-list.less | 91 ++++++++++----- package.json | 1 + spec-inbox/stores/draft-store-spec.coffee | 2 +- src/flux/models/contact.coffee | 6 + src/flux/models/utils.coffee | 1 - 12 files changed, 264 insertions(+), 146 deletions(-) diff --git a/internal_packages/message-list/lib/message-item.cjsx b/internal_packages/message-list/lib/message-item.cjsx index 2c5babd08..8528726b6 100644 --- a/internal_packages/message-list/lib/message-item.cjsx +++ b/internal_packages/message-list/lib/message-item.cjsx @@ -22,7 +22,7 @@ MessageItem = React.createClass # keyed by a fileId. The value is the downloadData. downloads: FileDownloadStore.downloadsForFileIds(@props.message.fileIds()) showQuotedText: @_messageIsEmptyForward() - collapsed: @props.collapsed + detailedHeaders: false componentDidMount: -> @_storeUnlisten = FileDownloadStore.listen(@_onDownloadStoreChange) @@ -38,8 +38,13 @@ MessageItem = React.createClass attachments =
{attachments}
header = -
- +
+ + @setState detailedHeaders: true} + isDetailed={@state.detailedHeaders} + date={@props.message.date} /> +
{ for Action in messageActions}
@@ -47,27 +52,27 @@ MessageItem = React.createClass @setState detailedHeaders: true} thread_participants={@props.thread_participants} + detailedParticipants={@state.detailedHeaders} message_participants={@props.message.participants()} /> + +
@setState detailedHeaders: false}> +
- if @state.collapsed -
-
- {header} -
-
- else -
-
- {header} - {attachments} - - {@_formatBody()} - - -
+
+
+ {header} + {attachments} + + {@_formatBody()} + +
+
_quotedTextClasses: -> React.addons.classSet "quoted-text-control": true @@ -122,7 +127,3 @@ MessageItem = React.createClass _onDownloadStoreChange: -> @setState downloads: FileDownloadStore.downloadsForFileIds(@props.message.fileIds()) - - _onToggleCollapsed: -> - @setState - collapsed: !@state.collapsed diff --git a/internal_packages/message-list/lib/message-list.cjsx b/internal_packages/message-list/lib/message-list.cjsx index 7de601f72..a9e7a0aa8 100755 --- a/internal_packages/message-list/lib/message-list.cjsx +++ b/internal_packages/message-list/lib/message-list.cjsx @@ -102,12 +102,6 @@ MessageList = React.createClass collapsed={collapsed} thread_participants={@_threadParticipants()} /> - # Start collapsing messages if we've loaded more than 15. This prevents - # us from trying to load an unbounded number of iframes until we have - # a better optimized message list. - if components.length > 10 - collapsed = true - components _onChange: -> diff --git a/internal_packages/message-list/lib/message-participants.cjsx b/internal_packages/message-list/lib/message-participants.cjsx index 41be6cc28..f870fa09f 100644 --- a/internal_packages/message-list/lib/message-participants.cjsx +++ b/internal_packages/message-list/lib/message-participants.cjsx @@ -6,39 +6,46 @@ MessageParticipants = React.createClass displayName: 'MessageParticipants' render: -> -
- {@_formattedParticipants()} + classSet = React.addons.classSet + "participants": true + "message-participants": true + "collapsed": not @props.detailedParticipants + +
+ {if @props.detailedParticipants then @_renderExpanded() else @_renderCollapsed()}
- _formattedParticipants: -> - - From: - {@_joinNames(@props.from)} - {if @_isToEveryone() then @_toEveryone() else @_toSome()} - - - _toEveryone: -> - + _renderCollapsed: -> + + {@_shortNames(@props.from)}  >  - Everyone - - - _toSome: -> - if @props.cc.length > 0 - cc_spans = - CC:  - {@_joinNames(@props.cc)} + {@_shortNames(@props.to)} + 0 then display:"inline" else display:"none"}> + Cc:  + {@_shortNames(@props.cc)} - -  >  - {@_joinNames(@props.to)} - {cc_spans} - _joinNames: (contacts=[]) -> + _renderExpanded: -> +
+
+
From: 
+
{@_fullContact(@props.from)}
+
+ +
+
To: 
+
{@_fullContact(@props.to)}
+
+ +
0 then display:"inline" else display:"none"}> +
Cc: 
+
{@_fullContact(@props.cc)}
+
+
+ + _shortNames: (contacts=[]) -> _.map(contacts, (c) -> c.displayFirstName()).join(", ") - _isToEveryone: -> - mp = _.map(@props.message_participants, (c) -> c.email) - tp = _.map(@props.thread_participants, (c) -> c.email) - mp.length > 10 and _.difference(tp, mp).length is 0 + _fullContact: (contacts=[]) -> + _.map(contacts, (c) -> c.displayFullContact()).join(", ") diff --git a/internal_packages/message-list/lib/message-timestamp.cjsx b/internal_packages/message-list/lib/message-timestamp.cjsx index 77db56d3b..ef39effe3 100644 --- a/internal_packages/message-list/lib/message-timestamp.cjsx +++ b/internal_packages/message-list/lib/message-timestamp.cjsx @@ -1,4 +1,4 @@ -moment = require 'moment' +moment = require 'moment-timezone' React = require 'react' module.exports = @@ -7,26 +7,37 @@ MessageTimestamp = React.createClass propTypes: date: React.PropTypes.object.isRequired, className: React.PropTypes.string, + isDetailed: React.PropTypes.bool + onClick: React.PropTypes.func render: -> -
{moment(@props.date).format(@_timeFormat())}
+
{@_formattedDate()}
+ + _formattedDate: -> + moment.tz(@props.date, @_currentTimezone()).format(@_timeFormat()) _timeFormat: -> - today = moment(@_today()) - dayOfEra = today.dayOfYear() + today.year() * 365 - msgDate = moment(@props.date) - msgDayOfEra = msgDate.dayOfYear() + msgDate.year() * 365 - diff = dayOfEra - msgDayOfEra - if diff < 1 - return "h:mm a" - if diff < 4 - return "MMM D, h:mm a" - else if diff > 1 and diff <= 365 - return "MMM D" + if @props.isDetailed + return "ddd, MMM Do YYYY, h:mm:ss a z" else - return "MMM D YYYY" + today = moment(@_today()) + dayOfEra = today.dayOfYear() + today.year() * 365 + msgDate = moment(@props.date) + msgDayOfEra = msgDate.dayOfYear() + msgDate.year() * 365 + diff = dayOfEra - msgDayOfEra + if diff < 1 + return "h:mm a" + if diff < 4 + return "MMM D, h:mm a" + else if diff > 1 and diff <= 365 + return "MMM D" + else + return "MMM D YYYY" # Stubbable for testing. Returns a `moment` - _today: -> moment() + _today: -> moment.tz(@_currentTimezone()) + + _currentTimezone: -> Intl.DateTimeFormat().resolvedOptions().timeZone diff --git a/internal_packages/message-list/spec/message-item-spec.cjsx b/internal_packages/message-list/spec/message-item-spec.cjsx index 5a3b3a4a9..9fbc24f43 100644 --- a/internal_packages/message-list/spec/message-item-spec.cjsx +++ b/internal_packages/message-list/spec/message-item-spec.cjsx @@ -66,6 +66,8 @@ EmailFrameStub = React.createClass({render: ->
}) MessageItem = proxyquire '../lib/message-item.cjsx', './email-frame': EmailFrameStub +MessageTimestamp = require '../lib/message-timestamp.cjsx' + describe "MessageItem", -> beforeEach -> @@ -98,7 +100,7 @@ describe "MessageItem", -> namespaceId: "nsid" @threadParticipants = [user_1, user_2, user_3, user_4] - + # Generate the test component. Should be called after @message is configured # for the test, since MessageItem assumes attributes of the message will not # change after getInitialState runs. @@ -110,16 +112,31 @@ describe "MessageItem", -> collapsed={collapsed} thread_participants={@threadParticipants} /> ) - - describe "when collapsed", -> + + # TODO: We currently don't support collapsed messages + # describe "when collapsed", -> + # beforeEach -> + # @createComponent({collapsed: true}) + # + # it "should not render the EmailFrame", -> + # expect( -> ReactTestUtils.findRenderedComponentWithType(@component, EmailFrameStub)).toThrow() + # + # it "should have the `collapsed` class", -> + # expect(@component.getDOMNode().className.indexOf('collapsed') >= 0).toBe(true) + + describe "when displaying detailed headers", -> beforeEach -> - @createComponent({collapsed: true}) + @createComponent({collapsed: false}) + @component.setState detailedHeaders: true - it "should not render the EmailFrame", -> - expect( -> ReactTestUtils.findRenderedComponentWithType(@component, EmailFrameStub)).toThrow() + it "correctly sets the participant states", -> + participants = ReactTestUtils.findRenderedDOMComponentWithClass(@component, "expanded-participants") + expect(participants).toBeDefined() + expect(-> ReactTestUtils.findRenderedDOMComponentWithClass(@component, "collapsed-participants")).toThrow() - it "should have the `collapsed` class", -> - expect(@component.getDOMNode().className.indexOf('collapsed') >= 0).toBe(true) + it "correctly sets the timestamp", -> + ts = ReactTestUtils.findRenderedComponentWithType(@component, MessageTimestamp) + expect(ts.props.isDetailed).toBe true describe "when not collapsed", -> beforeEach -> diff --git a/internal_packages/message-list/spec/message-participants-spec.cjsx b/internal_packages/message-list/spec/message-participants-spec.cjsx index 8b48ef3fc..75c384e40 100644 --- a/internal_packages/message-list/spec/message-participants-spec.cjsx +++ b/internal_packages/message-list/spec/message-participants-spec.cjsx @@ -1,24 +1,25 @@ _ = require 'underscore-plus' React = require "react/addons" +ReactTestUtils = React.addons.TestUtils TestUtils = React.addons.TestUtils {Contact, Message} = require "inbox-exports" MessageParticipants = require "../lib/message-participants.cjsx" user_1 = name: "User One" - email: "user1@inboxapp.com" + email: "user1@nilas.com" user_2 = name: "User Two" - email: "user2@inboxapp.com" + email: "user2@nilas.com" user_3 = name: "User Three" - email: "user3@inboxapp.com" + email: "user3@nilas.com" user_4 = name: "User Four" - email: "user4@inboxapp.com" + email: "user4@nilas.com" user_5 = name: "User Five" - email: "user5@inboxapp.com" + email: "user5@nilas.com" many_users = (new Contact({name: "User #{i}", email:"#{i}@app.com"}) for i in [0..100]) @@ -54,34 +55,74 @@ thread2_participants = [ ] describe "MessageParticipants", -> - it "determines the message is to everyone", -> - p1 = TestUtils.renderIntoDocument( - - ) - expect(p1._isToEveryone()).toBe true + describe "when collapsed", -> + beforeEach -> + @participants = TestUtils.renderIntoDocument( + + ) - it "knows when the message isn't to everyone due to participant mismatch", -> - p2 = TestUtils.renderIntoDocument( - - ) - # this should be false because we don't count bccs - expect(p2._isToEveryone()).toBe false + it "renders into the document", -> + participants = ReactTestUtils.findRenderedDOMComponentWithClass(@participants, "collapsed-participants") + expect(participants).toBeDefined() - it "knows when the message isn't to everyone due to participant size", -> - p2 = TestUtils.renderIntoDocument( - - ) - # this should be false because we don't count bccs - expect(p2._isToEveryone()).toBe false + it "uses short names", -> + to = ReactTestUtils.findRenderedDOMComponentWithClass(@participants, "to-contact") + expect(to.getDOMNode().innerHTML).toBe "User" + + describe "when expanded", -> + beforeEach -> + @participants = TestUtils.renderIntoDocument( + + ) + + it "renders into the document", -> + participants = ReactTestUtils.findRenderedDOMComponentWithClass(@participants, "expanded-participants") + expect(participants).toBeDefined() + + it "uses full names", -> + to = ReactTestUtils.findRenderedDOMComponentWithClass(@participants, "to-contact") + expect(to.getDOMNode().innerHTML).toBe "User Two <user2@nilas.com>" + + + # TODO: We no longer display "to everyone" + # + # it "determines the message is to everyone", -> + # p1 = TestUtils.renderIntoDocument( + # + # ) + # expect(p1._isToEveryone()).toBe true + # + # it "knows when the message isn't to everyone due to participant mismatch", -> + # p2 = TestUtils.renderIntoDocument( + # + # ) + # # this should be false because we don't count bccs + # expect(p2._isToEveryone()).toBe false + # + # it "knows when the message isn't to everyone due to participant size", -> + # p2 = TestUtils.renderIntoDocument( + # + # ) + # # this should be false because we don't count bccs + # expect(p2._isToEveryone()).toBe false diff --git a/internal_packages/message-list/spec/message-timestamp-spec.cjsx b/internal_packages/message-list/spec/message-timestamp-spec.cjsx index 57894f212..069b2668f 100644 --- a/internal_packages/message-list/spec/message-timestamp-spec.cjsx +++ b/internal_packages/message-list/spec/message-timestamp-spec.cjsx @@ -11,8 +11,7 @@ describe "MessageTimestamp", -> @item = TestUtils.renderIntoDocument( ) - @itemNode = @item.getDOMNode() - + # test messsage time is 1415814587 it "displays the time from messages LONG ago", -> spyOn(@item, "_today").andCallFake -> testDate().add(2, 'years') @@ -33,3 +32,10 @@ describe "MessageTimestamp", -> it "displays the time from messages recently", -> spyOn(@item, "_today").andCallFake -> testDate().add(2, 'hours') expect(@item._timeFormat()).toBe "h:mm a" + + it "displays the full time when in detailed timestamp mode", -> + itemDetailed = TestUtils.renderIntoDocument( + + ) + spyOn(itemDetailed, "_today").andCallFake -> testDate() + expect(itemDetailed._timeFormat()).toBe "ddd, MMM Do YYYY, h:mm:ss a z" diff --git a/internal_packages/message-list/stylesheets/message-list.less b/internal_packages/message-list/stylesheets/message-list.less index 743f74b9b..eb6a3eb63 100644 --- a/internal_packages/message-list/stylesheets/message-list.less +++ b/internal_packages/message-list/stylesheets/message-list.less @@ -72,6 +72,7 @@ padding: @spacing-standard @spacing-double; .message-header { + position: relative; font-size: @font-size-small; .message-actions { float:right; @@ -88,41 +89,18 @@ .message-time, .message-indicator { color: @text-color-very-subtle; - margin-top: 3px; float: right; + margin-left: 1em; + + &:hover { + cursor: pointer; + } } .message-indicator { margin-right: 4px; } - .to-label { - font-weight: 600; - } - - .to-label, .to-everyone, .cc-label, .cc-contact, .to-label, .to-contact { - position: relative; - top: -1px; - } - - .from-label { - display: none; - } - .from-contact { - font-weight: @headings-font-weight; - color: @text-color; - } - - .to-label, .cc-label { - color: @text-color-very-subtle; - } - .cc-label { - margin-left: @spacing-standard; - } - .to-contact, .cc-contact, .to-everyone { - color: @text-color-very-subtle; - } - iframe { width: 100%; border: 0; @@ -131,7 +109,64 @@ overflow: auto; } } + + + .collapse-headers { + position: absolute; + bottom: 0; + right: 0; + color: @text-color-very-subtle; + } + + .collapse-headers:hover {cursor: pointer;} + } .attachments-area { padding-top: @spacing-standard; } + + +/////////////////////////////// +// message-participants.cjsx // +/////////////////////////////// +.message-participants { + + &.collapsed:hover {cursor: pointer;} + + .from-contact { + font-weight: @headings-font-weight; + color: @text-color; + } + + .to-label, .cc-label { + color: @text-color-very-subtle; + } + .cc-label { + margin-left: @spacing-standard; + } + .to-contact, .cc-contact, .to-everyone { + color: @text-color-very-subtle; + } + + .to-label { font-weight: 600; } + + .expanded-participants { + position: relative; + padding-right: 1.2em; + + .from-label, .to-label, .cc-label { + float: left; + display: block; + font-weight: 600; + margin-left: 0; + } + + .to-label, .cc-label { + margin-right: 1.15em; + } + + .from-contact, .to-contact, .cc-contact { + padding-left: 2.85em; + } + } +} diff --git a/package.json b/package.json index efde07a79..b58faa3ff 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "marked": "^0.3", "mkdirp": "^0.5", "moment": "^2.8", + "moment-timezone": "^0.3", "nslog": "^2.0.0", "oniguruma": "^4.0.0", "optimist": "0.4.0", diff --git a/spec-inbox/stores/draft-store-spec.coffee b/spec-inbox/stores/draft-store-spec.coffee index d1913f421..caba22aa1 100644 --- a/spec-inbox/stores/draft-store-spec.coffee +++ b/spec-inbox/stores/draft-store-spec.coffee @@ -193,4 +193,4 @@ describe "DraftStore", -> @_callNewMessageWithContext {threadId: fakeThread.id, messageId: fakeMessage1.id} , (thread, message) -> expect(message).toEqual(fakeMessage1) - {} \ No newline at end of file + {} diff --git a/src/flux/models/contact.coffee b/src/flux/models/contact.coffee index 10efd9725..561a237c4 100644 --- a/src/flux/models/contact.coffee +++ b/src/flux/models/contact.coffee @@ -21,6 +21,12 @@ class Contact extends Model json['name'] ||= json['email'] json + displayFullContact: -> + if @name and @name isnt @email + return "#{@name} <#{@email}>" + else + return @email + displayName: -> return "You" if @email == NamespaceStore.current().emailAddress @_nameParts().join(' ') diff --git a/src/flux/models/utils.coffee b/src/flux/models/utils.coffee index 68a508197..109f7570c 100644 --- a/src/flux/models/utils.coffee +++ b/src/flux/models/utils.coffee @@ -88,7 +88,6 @@ Utils = Utils.images = {} Utils.images[path.basename(file)] = file for file in files - # console.log("Loaded image map in #{Date.now()-start}msec") if window.devicePixelRatio > 1 return Utils.images["#{name}@2x.#{ext}"] ? Utils.images[fullname] ? Utils.images["#{name}@1x.#{ext}"]