This applies to both the link-generator and link-clicker display. We've got the basic elements, but we need to style them as per the UX. See the user story for details.

svg files on bug 1138445.

Comment on attachment 8626185 [details] [diff] [review] Style text chat elements and add timestamps Review of attachment 8626185 [details] [diff] [review]: ----------------------------------------------------------------- Here's an initial set of comments. There's a few eslint errors raised: content/shared/js/textChatView.jsx 25:6 error Prop types declarations should be sorted alphabetically react/jsx-sort-prop-types 26:6 error Prop types declarations should be sorted alphabetically react/jsx-sort-prop-types 69:13 error Missing parentheses around multilines JSX react/wrap-multilines 198:23 error Missing parentheses around multilines JSX react/wrap-multilines 199:38 error Props should be sorted alphabetically react/jsx-sort-props 202:38 error Props should be sorted alphabetically react/jsx-sort-props ::: browser/components/loop/content/shared/css/conversation.css @@ +1520,5 @@ > +} > + > +.sent p, > +.received p { > + background: #fff; The selectors for this aren't very efficient and they could apply to other things not in the text-chat-entry tree... can you use .text-chat-entry > p ? @@ +1524,5 @@ > + background: #fff; > +} > + > +.text-chat-entry.sent > p { > + border-bottom-right-radius: 0; This needs a html[dir="rtl"] equivalent for left-radius and to reset the right radius. There's a checkbox at the top of the ui-showcase for rtl mode where you can check/test these. @@ +1528,5 @@ > + border-bottom-right-radius: 0; > +} > + > +.text-chat-entry.received > p { > + border-top-left-radius: 0; Ditto wrt rtl. @@ +1559,5 @@ > + align-self: center; > +} > + > +/* Sent text chat entries should be on the right */ > +.sent { nit: again, these should apply to more specific items that just .sent or .received. @@ +1563,5 @@ > +.sent { > + justify-content: flex-end; > +} > + > +.received .text-chat-entry-timestamp { Can we use the more efficient child selector please. @@ +1578,5 @@ > + background: #fff; > + content: ""; > +} > + > +.text-chat-entry.sent > p:after { rtl modes needed for this and the .received equivalents. ::: browser/components/loop/content/shared/js/textChatView.jsx @@ +27,5 @@ > + }, > + > + getDefaultProps: function() { > + return { > + timestamp: (new Date()).toISOString() If timestamp is required, I don't think we should be defaulting it here. @@ +52,5 @@ > + > + /** > + * Pretty print timestamp. From time in milliseconds to HH:MM > + */ > + _renderTimestamp: function() { Can you put this above the render() function - we generally keep render() last as its easier to find. @@ +55,5 @@ > + */ > + _renderTimestamp: function() { > + var date = new Date(this.props.timestamp); > + var minutes = date.getMinutes(); > + var hours = date.getHours(); nit: no need to align = here. @@ +66,5 @@ > + hours = "0" + hours; > + } > + > + return <span className="text-chat-entry-timestamp"> > + {hours}:{minutes} The problem with using this is that it isn't going to adjust per a locale. I would suggest using something like: new Date().toLocaleTimeString(navigator.mozL10n.language.code, {hour: "numeric", minute: "numeric", hour12: false}) ::: browser/components/loop/jar.mn @@ +40,5 @@ > # one? > content/browser/loop/shared/img/spinner.png (content/shared/img/spinner.png) > content/browser/loop/shared/img/spinner@2x.png (content/shared/img/spinner@2x.png) > + content/browser/loop/shared/img/chatbubble-arrow-left.svg (content/shared/img/chatbubble-arrow-left.svg) > + content/browser/loop/shared/img/chatbubble-arrow-right.svg (content/shared/img/chatbubble-arrow-right.svg) Can yuou align the ( with the ones above/below please ::: browser/components/loop/test/shared/textChatView_test.js @@ +131,5 @@ > + React.createElement(loop.shared.views.chat.TextChatEntry, props)); > + } > + > + beforeEach(function() { > + }); nit: empty function. @@ +205,5 @@ > + expect(node.querySelectorAll(".text-chat-entry-timestamp").length) > + .to.eql(1); > + }); > + > + it("should have a correctly formatted timestamp", function() { I'm not sure we actually need to test this - this is more a function of the ui-showcase (especially when we change to localised values).

Comment on attachment 8626302 [details] [diff] [review] Style text chat elements and add timestamps Review of attachment 8626302 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the comments addressed. ::: browser/components/loop/content/shared/css/conversation.css @@ +1518,5 @@ > + align-self: auto; > +} > + > +.sent > p, > +.received > p { Please add additional scoping e.g. to .text-chat-entry.sent (several places in the file). ::: browser/components/loop/content/shared/js/actions.js @@ +186,5 @@ > */ > ReceivedTextChatMessage: Action.define("receivedTextChatMessage", { > contentType: String, > + message: String, > + receivedTimestamp: String nit: please add in the fact that sentTimestamp is optional here. ::: browser/components/loop/test/shared/textChatView_test.js @@ +268,5 @@ > + sentTimestamp: "1970-01-01T00:02:00.000Z", > + receivedTimestamp: "1970-01-01T00:02:00.000Z" > + }); > + > + fakeClock.tick(1000 * 60); I don't think you need this - given that you're specifying timestamps on all the entries.

I did some exploratory testing across platforms (Windows 7 64-bit, Mac OS X 10.10.4 and Ubuntu 14.04 32-bit) using latest Aurora 41.0a2 and can confirm that the time layout changes and timestamp are visible and correctly displayed.