Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Bug 1399276, adds new whatsnew page for dev edition 57 #5132

Merged
merged 1 commit into from
Sep 26, 2017
Merged

Fix Bug 1399276, adds new whatsnew page for dev edition 57 #5132

merged 1 commit into from
Sep 26, 2017

Conversation

schalkneethling
Copy link

Description

Add new whatsnew page for Dev edition 57 located at /firefox/57.0a2/whatsnew/

Issue / Bugzilla link

https://bugzilla.mozilla.org/show_bug.cgi?id=1399276

Testing

Basic responsiveness.
Firefox only.

Copy link
Member

@craigcook craigcook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just some superficial nitpicks and updating some graphics. I'll make a new background for the top section and add it to the bug.

&:before {
display: block;
content: '';
@include at2x('/media/img/firefox/products/developer/quantum/dev-edition-logo.png', 105px, 108px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the wrong logo and was probably just a placeholder in the design mockup. The final logo is /media/img/logos/firefox/logo-developer-quantum.png

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that is what I was thinking as well. Completely escaped me to ask in the end. Will use the one you pointed at here.

@media #{$mq-tablet} {
@include background-size(1440px 1034px);
background-image: url('/media/img/firefox/products/developer/quantum/whatsnew-hero-background.png');
background-position: 0 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be centered horizontally (background-position: center bottom;)

Also in any window wider than 1440px the "blobs" are cut off with a hard edge. I made a background that blended out to white for the product page and I can send you a version without the blue wave on the bottom to use here.


@media #{$mq-desktop} {
@include at2x('/media/img/firefox/products/developer/quantum/whatsnew-hero-background.png', 1780px, 1278px);
background-position: 0 -169px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Centered here, too, and probably anchored to the bottom to better handle vertical expansion, though that makes the positioning trickier... Maybe just leave it as center bottom and let the tops of the blobs be cut off.

}

h3 {
@include font-size(18px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use the @include font-size-level5; mixin here without the other media queries.


@media #{$mq-phone-wide} {
padding: .5em 18px;
@include font-size(13px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary with the level5 mixin.

}

.section-body {
@include at2x('/media/img/firefox/template/nightly-logo.png', 165px, 165px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This image is 180 x 186 so giving it square dimensions slightly squishes it. I think we should put a squared version of the Nightly logo in /img/logos/firefox to be the "canonical" nightly logo.

}

@media #{$mq-desktop} {
@include background-size(300px 300px);
Copy link
Member

@craigcook craigcook Sep 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The low-res logo is 180 x 186 so it's being scaled up on non-retina screens and looks blurry. But if we switch to a different graphic we can make it 300 x 300 (and 600 x 600 at 2x).

}

h3 {
@include font-size(18px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're trying to avoid arbitrary font sizes and stick to a set scale (alas, not properly documented anywhere yet, but the example is at http://craigcook.github.io/etc/typescale/).

H3 elements should be level3 by default so you may not need to declare any other sizing here but if you do, try to use one of the mixins. If you can't use a mixin, at least use sizes within the defined scale (12px, 14px, 16px, 18px, 24px, 36px, 48px, 60px, 72px). This is another place where I'm willing to disregard the mockup and impose some consistency of our own.

<li>
<img class="icon" src="{{ static('img/firefox/products/developer/quantum/devtools-icon.svg') }}" alt="">
<h3>{{ _('Firefox Devtools') }}</h3>
<p class="cta"><a href="#devtools" class="button button-hollow">{{ _('Learn more') }}</a></p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use smooth scrolling for these?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scroll-behavior: smooth; CSS FTW 👍

<img class="hero-image" src="{{ static('img/firefox/products/developer/quantum/stylo-engine.svg') }}" alt="">
<p>
{% trans %}
Firefox Quantum includes a new CSS engine, written in Rust, that has state-of-the-art innovations and is blazingly fast.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should put a max-width on this blurb. It's such a short sentence, I get "fast." on its own line in a wide window. It'll be different for each locale but I think something like max-width: $width-phone-wide; margin: 0 auto 1.25em; might not hurt.

@schalkneethling
Copy link
Author

Thanks for the review @craigcook - updated.

Copy link
Member

@craigcook craigcook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few tiny nitpicks, no real show-stoppers.

When I look at the high res Nightly logo at full size it's a little blurry and looks like it was upscaled from the smaller one, though it looks alright when it's scaled back down on the page.

}

@media #{$mq-desktop} {
@include at2x('/media/img/firefox/products/developer/quantum/whatsnew-firstrun-hero-background.png', 3400px, 900px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: shouldn't need to redeclare the whole thing here, just the new background-size.

&:before {
display: block;
content: '';
@include at2x('/media/img/logos/firefox/logo-developer-quantum.png', 105px, 108px);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logo graphic is square so width and height should be the same here.

@craigcook
Copy link
Member

r+! Just needs a squash

@craigcook craigcook merged commit 4595663 into mozilla:master Sep 26, 2017
@flodolo flodolo removed the L10N label Sep 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants