Closed Bug 1218405 Opened 4 years ago Closed 4 years ago

Change the standalone background for the visual refresh/latest designs

Categories

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

defect
Points:
2

Tracking

(firefox44 verified)

VERIFIED FIXED
mozilla44
Iteration:
44.3 - Nov 2
Tracking Status
firefox44 --- verified

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file, 1 obsolete file)

The standalone background needs changing per the mock-ups. This is needed for a release fairly soon, as the contrast of the Hello logo and the background is a bit strange.

Alongside this, there's a few extra fixes:

- Make the logo a little bit smaller, and don't repeat the background
- Invert the help icon
- Change the colour of the info-area text
Comment on attachment 8678879 [details] [diff] [review]
Change Loop's standalone background for the visual refresh/latest designs.

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

::: browser/components/loop/content/shared/img/svg/glyph-help-16x16.svg
@@ +1,1 @@
> +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><circle fill="#fff" cx="8" cy="8" r="8"/><path fill="#5a5a5a" d="M10.716 5.643c0 1.943-2.158 1.812-2.158 3.154v.3H6.83v-.37C6.83 6.65 8.74 6.793 8.74 5.81c0-.43-.312-.683-.84-.683-.49 0-.972.24-1.403.73l-1.21-.934C5.966 4.12 6.854 3.64 8.09 3.64c1.75 0 2.626.936 2.626 2.003zm-1.92 5.625c0 .6-.478 1.092-1.078 1.092s-1.08-.492-1.08-1.092c0-.588.48-1.08 1.08-1.08s1.08.492 1.08 1.08z"/></svg>
\ No newline at end of file

I notice that this version no longer has the initial line with the encoding="utf-8".  Not sure if that matters, but calling it out just to be sure it's intentional.

::: browser/components/loop/standalone/content/css/webapp.css
@@ +59,5 @@
>  
>  .standalone-overlay-wrapper > .hello-logo {
> +  width: 120px;
> +  height: 19px;
> +  background: url("../shared/img/hello_logo.svg") 0%/contain no-repeat;

The / syntax is unusual enough that it took me a good 10s mins of poking around on MDN to find the documentation showing me how to review it.  Is there some reason not to just leave this as a separate, more obviously readable background-size property?

It's still not clear to me what the 0%/contain is supposed to represent.  If I'm reading https://developer.mozilla.org/en-US/docs/Web/CSS/background-size correctly, it kinda looks like "contain" can be used by itself, or as part of multiple images, but this usage doesn't look like either of those.
(In reply to Dan Mosedale (:dmose) - use needinfo flag for response from comment #2)
> Comment on attachment 8678879 [details] [diff] [review]
> Change Loop's standalone background for the visual refresh/latest designs.
> 
> Review of attachment 8678879 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/components/loop/content/shared/img/svg/glyph-help-16x16.svg
> @@ +1,1 @@
> > +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><circle fill="#fff" cx="8" cy="8" r="8"/><path fill="#5a5a5a" d="M10.716 5.643c0 1.943-2.158 1.812-2.158 3.154v.3H6.83v-.37C6.83 6.65 8.74 6.793 8.74 5.81c0-.43-.312-.683-.84-.683-.49 0-.972.24-1.403.73l-1.21-.934C5.966 4.12 6.854 3.64 8.09 3.64c1.75 0 2.626.936 2.626 2.003zm-1.92 5.625c0 .6-.478 1.092-1.078 1.092s-1.08-.492-1.08-1.092c0-.588.48-1.08 1.08-1.08s1.08.492 1.08 1.08z"/></svg>
> \ No newline at end of file
> 
> I notice that this version no longer has the initial line with the
> encoding="utf-8".  Not sure if that matters, but calling it out just to be
> sure it's intentional.

This is what svgo produced as output, so I'm going to assume it doesn't really matter.

> ::: browser/components/loop/standalone/content/css/webapp.css
> @@ +59,5 @@
> >  
> >  .standalone-overlay-wrapper > .hello-logo {
> > +  width: 120px;
> > +  height: 19px;
> > +  background: url("../shared/img/hello_logo.svg") 0%/contain no-repeat;
> 
> The / syntax is unusual enough that it took me a good 10s mins of poking
> around on MDN to find the documentation showing me how to review it.  Is
> there some reason not to just leave this as a separate, more obviously
> readable background-size property?

Yeah we can keep it separate.

> It's still not clear to me what the 0%/contain is supposed to represent.

Its basically `background-position: 0%; background-size: contain`.
Attachment #8678879 - Attachment is obsolete: true
Attachment #8678879 - Flags: review?(dmose)
Attachment #8678897 - Flags: review?(dmose)
Comment on attachment 8678897 [details] [diff] [review]
Change Loop's standalone background for the visual refresh/latest designs. v2

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

Looks good, r=dmose
Attachment #8678897 - Flags: review?(dmose) → review+
https://hg.mozilla.org/mozilla-central/rev/582bfbe86c0d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Flags: qe-verify+
QA Contact: bogdan.maris
Verified that the Standalone background is the same as mockups point and also the extra fixes from comment 0 look good across platforms (Windows 10 64-bit, Windows 7 64-bit, Mac OS X 10.11.1 and Ubuntu 14.04 32-bit) using latest Developer Edition 44.0a2.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.