The text-entry field needs styling for text chat on both standalone & desktop displays. See the user story for more information.

Based on top of patch in bug 1168848; also fixes FramedExample to handle dashed examples correctly and adds dashes in various places so that one can see where the TextChatView borders are.

Note that this does turn on the text chat view for all of the various full views in the showcase, but, given where we are in development, that seems reasonable to me.

This version updated to populate the message list and remove a regression. It turns out we're going to need to fix bug 1173909 for this to be very useful, so I'm going to do that before requesting review.

This needs more post-rebase cleanup; should be straightforward.

Comment on attachment 8623852 [details] [diff] [review] Make TextChat style correctly when focussed I see how to make the scrolling simulate the real app code flow better; new patch forthcoming.

Comment on attachment 8623852 [details] [diff] [review] Make TextChat style correctly when focussed Review of attachment 8623852 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/content/shared/css/conversation.css @@ +1308,5 @@ > border-top: 1px solid #999; > } > > +/* turn the visible border blue as a visual indicator of focus */ > +.fx-embedded .text-chat-box > form > input:focus { I'm pretty sure this shouldn't be desktop only. Please check with UX if unsure.

This patch turns on text-chat in the ui-showcase, in addition to fixing the style user-story here. It also a few other minor tweaks As part of landing, I'll file a bug about fixing the "invitiation" view that this leaves in an odd state...

Comment on attachment 8624441 [details] [diff] [review] Make TextChat style correctly when focussed Review of attachment 8624441 [details] [diff] [review]: ----------------------------------------------------------------- When you file the bug on the invitation display, please can you extend it to find a solution to display different states of the text chat store - especially on desktop, we should have a "text chat enabled but no entries visible" option (ideally we'd have variants for standalone as well, e.g. text chat not enabled). ::: browser/components/loop/content/shared/css/conversation.css @@ +1499,5 @@ > > +/* turn the visible border blue as a visual indicator of focus */ > +.text-chat-box > form > input:focus { > + border: 0; > + border-top: 1px solid #66c9f2; I think this needs ux-review/input. On desktop I can barely see the difference especially when the entries field is collapsed. Whilst this is as-designed, I'm not sure its good enough. On Standalone, there's a 1px border all the way around that disappears (except for the top border) when you click in the element. If the placeholder is in there, then that shifts position as well. ::: browser/components/loop/ui/ui-showcase.jsx @@ +252,5 @@ > sdkDriver: mockSDK > }); > > textChatStore.setStoreState({ > + textChatEnabled: true, Please can you move the updateRoomInfo to before this section, so that the showcase displays more like what's expected (it'd be extremely unlikely that we'd get updateRoomInfo in after text messages - and there's a bug on file somewhere that will move us to ensure that we do).

(In reply to Mark Banner (:standard8) from comment #9) > When you file the bug on the invitation display, please can you extend it to > find a solution to display different states of the text chat store - > especially on desktop, we should have a "text chat enabled but no entries > visible" option (ideally we'd have variants for standalone as well, e.g. > text chat not enabled). See bug 1176339. > ::: browser/components/loop/content/shared/css/conversation.css > @@ +1499,5 @@ > > > > +/* turn the visible border blue as a visual indicator of focus */ > > I think this needs ux-review/input. > > On desktop I can barely see the difference especially when the entries field > is collapsed. > > Whilst this is as-designed, I'm not sure its good enough. Had a Hello call with Sevaan about this, he pointed out that this is intended to be a subtler cue than an input box that requires input, since it's unclear when/if you'll type in this one again. He suggested leaving it as designed for now, and he'll talk to Vicky about other options. > On Standalone, there's a 1px border all the way around that disappears > (except for the top border) when you click in the element. If the > placeholder is in there, then that shifts position as well. This is actually a separate bug: looking closely at the design, there shouldn't be 1px border all the way around in the unfocussed state ever. I've fixed this in the new iteration. > ::: browser/components/loop/ui/ui-showcase.jsx > @@ +252,5 @@ > > sdkDriver: mockSDK > > }); > > > > textChatStore.setStoreState({ > > + textChatEnabled: true, > > Please can you move the updateRoomInfo to before this section, so that the > showcase displays more like what's expected (it'd be extremely unlikely that > we'd get updateRoomInfo in after text messages - and there's a bug on file > somewhere that will move us to ensure that we do). Fixed. This forced me to move to using actions rather than mucking about in the store directly, which is less fragile anyway.

Comment on attachment 8624874 [details] [diff] [review] Make TextChat style correctly when focussed Review of attachment 8624874 [details] [diff] [review]: ----------------------------------------------------------------- Ok, looks good. r=Standard8 with the comment addressed. ::: browser/components/loop/ui/ui-showcase.jsx @@ +296,3 @@ > // Update the text chat store with the room info. > textChatStore.updateRoomInfo(new sharedActions.UpdateRoomInfo({ > + roomName: "A Very Long Conversation Name", This section doesn't need an extra indentation.

Dan: I pushed this for you with my comment addressed, so that I could push the fix to bug 1176278.

Awesome; thanks!

Tested using Firefox Developer Edition 41.0a2 and https://loop-dev.stage.mozaws.net/v0 across platforms (Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit) and I can confirm the implementation is according to user story, found no new issue so I'll mark this as verified fixed.