1226607 - When there's no converstions - the UX isn't matching new user journey (see URL)

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Paul Silaghi, QA [:pauly]]

Attachment: beta 43.png

FF 43b5, 45.0a1 (2015-11-20) Ubuntu 14.04 x64
STR:
1. Open the Hello panel
2. Make sure there are no conversations in list

Actual results:
"N" character from the "No conversation yet" is cropped

Notes: This is Linux specific, doesn't reproduce on Mac, Win.
Selecting the entire word with mouse makes the character to be completely displayed until I close/reopen the Hello panel.
This is more prominent in FF 43, compared to FF 45.

Comment 1

[Paul Silaghi, QA [:pauly]]

Attachment: nightly 45.png

Comment 2

[Paul Silaghi, QA [:pauly]]

First bad nightly: 2015-09-13

Comment 3

[:shell escalante]

UX mismatch in 45 from what it should be.  Will let it go in 43 as it's changing drastically.

http://people.mozilla.org/~sfranks/Hello/Firefox%20Hello%20Link%20Generator.png

Comment 4

[GitHub Autolander Bot]

Attachment: [loop] chrafuse:bug-1226607-no-converstions-UX-is-not-matching-new-user-journey > mozilla:master

Comment 5

[Chris Rafuse :crafuse]

Attachment: Link to Github pull-request: https://github.com/mozilla/loop/pull/36

Just needed to add a height to the paragraph container and may have been causing some font scaling.

Comment 6

[Ed Lee :Mardak]

Comment on attachment 8703037 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/36

The pull request doesn't seem to do what this bug is requesting of matching the new user journey, which doesn't refer to conversations.

Comment 7

[Ed Lee :Mardak]

Attachment: design cropped from linked url

I believe this is the relevant portion of the linked url as per bug title: http://people.mozilla.org/~sfranks/Hello/Firefox%20Hello%20Link%20Generator.png

Comment 8

[Chris Rafuse :crafuse]

Comment on attachment 8703037 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/36

Removed view, CSS, tests and image.

Comment 9

[Ed Lee :Mardak]

Can you attach a screenshot either here or in github? The pull request seems like it wouldn't match the requested design.

Comment 10

[Chris Rafuse :crafuse]

Attachment: Empty Conversations Panel Content

Added screen shot of no conversations shown in panel.

Comment 11

[Chris Rafuse :crafuse]

The panel resize will be completed in Bug 1234183 as this will break functionality.
Removal of the no conversations message view was completed in this bug.
Created Bug 1236082 for footer email address display.

Comment 12

[Ed Lee :Mardak]

Comment on attachment 8703176 [details]
Empty Conversations Panel Content

This bug should focus on removing/not-showing content and bug 1234183 should focus on sizing. So, the fix here should result in not having the line or "Recently browsed" shown.

Comment 13

[Chris Rafuse :crafuse]

Attachment: noRoomsShown.png

Does not render the top border, rooms title and rooms list if there are no conversation rooms.

Comment 14

[Ed Lee :Mardak]

Comment on attachment 8703037 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/36

Added some minor issues for naming and spacing. Bigger question of why split it into multiple methods instead of a single _renderRoomsList helper.

Comment 15

[Ed Lee :Mardak]

Comment on attachment 8703880 [details]
noRoomsShown.png

Looks like UI showcase for panels might have been broken by bug 1219158?

Comment 16

[Ed Lee :Mardak]

(In reply to Ed Lee :Mardak from comment #15)
> Looks like UI showcase for panels might have been broken by bug 1219158?
Fixed by bug 1236917.

Comment 17

[Chris Rafuse :crafuse]

Comment on attachment 8703037 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/36

Due to eslint restrictions else can not be used in a function return, I assume it returns void if not returned.  Renamed the functions and fixed indentation of rooms list function.  I could not use the code in the same render function as it would break as soon as I added any conditionals.

Comment 18

[Ed Lee :Mardak]

(In reply to Chris Rafuse :crafuse from comment #17)
> Due to eslint restrictions else can not be used in a function return, I
> assume it returns void if not returned.
If you're running into no-else-return, then you should have done `if () return null; return <>` instead of `if () return null; else return <>;

By omitting the return, you're implicitly returning undefined.

Comment 19

[Ed Lee :Mardak]

(In reply to Chris Rafuse :crafuse from comment #17)
> I could not use the code in the same render function as it would break as soon as I added any conditionals.
How were you adding conditionals? The tertiary {length ? <h1></h1> : null} should work. See https://github.com/mozilla/loop/blob/master/add-on/panels/js/panel.jsx#L490

Comment 20

[Ed Lee :Mardak]

See comment 19 / PR comment https://github.com/mozilla/loop/pull/36#discussion_r49051911 before addressing other issues as putting the _renders inline avoids fixing some other issues.

Comment 21

[Ed Lee :Mardak]

Comment on attachment 8703037 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/36

r=Mardak with a note that the UI is slightly changed for the loading screen as there's no border under the button.

Comment 22

[Ed Lee :Mardak]

https://github.com/mozilla/loop/commit/cb2cabe52a94fdde46dcc4f6bd31955ce7110a50

Comment 23

[Paul Silaghi, QA [:pauly]]

No text when the list is empty in the new UI.
Verified fixed FF 46.0a1 (2016-01-20), Ubuntu 14.04.