1193666 - Context Tile/Room Name surround colour needs changing

Status: VERIFIED FIXED

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

Description

[Mark Banner (:standard8)]

The surround colour for the room name / context tile needs changing for the standalone + conversation window - currently its conflicting with that of the hover for the context.

Comment 1

[Mark Banner (:standard8)]

Attachment: Fix surrounding colour of context tiles for Loop's text chat views. Also fix the fallback icon for context on desktop.

This fixes the background colour, and it fixes the fallback icon that we use on desktop - when I'd implemented the media views, I forgot to change the useDesktopPaths item to use the passed in props.

I've also removed passing useDesktopPaths to TextChatRoomName as it didn't need it.

Comment 2

[Mark Banner (:standard8)]

Attachment: Screenshot of patch effects

Screenshot with left-to-right as follows:

- Current display
- Fixed surround colour and icon
- Fixed with Hover effect showing.

I'm just concentrating on the background & icon in this bug as we'll need to uplift these to 42.

Comment 3

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8646891 [details]
Screenshot of patch effects

I wouldn't have hover feedback on 2 elements, we either have the link underlining hover or the whole thing.  I would go for only the link but let's ask Sevaan as he might think differently.

THanks

Comment 4

[Sevaan Franks [:sevaan]]

Comment on attachment 8646891 [details]
Screenshot of patch effects

I like the tile colour background change on hover because it's such great visual feedback, and I think it's okay that the link underlines at the same time...because it's not two elements, really...we need to think of the tile as one whole element.

However, in this case the background colour does conflict with the background of the context area and it looks off.

In the future this won't matter, because the entire context area may be removed completely since any associated context will just be opened up as a tab in web sharing.

Until then though, I don't think a background change of the tile is necessary and we could just stick with the link underline on hover.

Vicky, what do you think about changing the border colour of the tile on hover if we leave the background white?

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8646890 [details] [diff] [review]
Fix surrounding colour of context tiles for Loop's text chat views. Also fix the fallback icon for context on desktop.

Will update tomorrow with responses from UX.

Comment 6

[Victoria Gerchinhoren [:vicky]]

(In reply to Sevaan Franks [:sevaan] from comment #4)
> Comment on attachment 8646891 [details]
> Screenshot of patch effects
> 
> I like the tile colour background change on hover because it's such great
> visual feedback, and I think it's okay that the link underlines at the same
> time...because it's not two elements, really...we need to think of the tile
> as one whole element.
> 
> However, in this case the background colour does conflict with the
> background of the context area and it looks off.
> 
> In the future this won't matter, because the entire context area may be
> removed completely since any associated context will just be opened up as a
> tab in web sharing.
> 
> Until then though, I don't think a background change of the tile is
> necessary and we could just stick with the link underline on hover.
> 
> Vicky, what do you think about changing the border colour of the tile on
> hover if we leave the background white?

Sevaan, that could be a fair solution, though, I am not sure how strong feedback will that be... 1px line, should change color drastically, right? I thought about the light blue competing there, but since hover is just a moment (and hopefully the hovering animation will be implemented soon), I don't see it unviable.

Comment 7

[Mark Banner (:standard8)]

Sevaan, any suggestion on the way forward here?

Comment 8

[Sevaan Franks [:sevaan]]

Attachment: Context Tile Hover State

(In reply to Mark Banner (:standard8) from comment #7)
> Sevaan, any suggestion on the way forward here?

Mock attached.

On hover let's keep the URL underline, change the board colour to the Hello Active Blue (#5CCCEE) and increase the border thickness by 2px.

Comment 9

[Mark Banner (:standard8)]

Attachment: Revised screenshots

Sevaan, how's this. Non-hover on the left, hover versions on the right.

Comment 10

[Mark Banner (:standard8)]

Attachment: Fix surrounding colour of context tiles for Loop's text chat views. Also fix the fallback icon for context on desktop.

Updated for ux-feedback, changes the hover settings and fixes the fallback icon that's used on desktop.

I've also removed passing useDesktopPaths to TextChatRoomName as it didn't need it.

Comment 11

[Mike de Boer [:mikedeboer]]

Comment on attachment 8652324 [details] [diff] [review]
Fix surrounding colour of context tiles for Loop's text chat views. Also fix the fallback icon for context on desktop.

Review of attachment 8652324 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Thanks :)

::: browser/components/loop/content/shared/css/common.css
@@ +586,5 @@
>  }
>  
>  .clicks-allowed.context-wrapper:hover {
> +  border: 2px solid #5cccee;
> +  padding: calc(.8em - 1px);

Well, there's a good explanation for this, but could you please document this above in a comment for future eyes?

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/1ad88439378f

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1ad88439378f

Comment 14

[Bogdan Maris, Desktop QA]

Verified the changes on Firefox Developer Edition 43.0a2 across platforms (Windows 7 64-bit, Windows 10 64bit, Mac OS X 10.11 and Ubuntu 14.04 32-bit) and no issues were found.