As a contingency plan, the attached spec outlines changes that would need to be made to the about:newtab tiles page: https://mozilla.invisionapp.com/share/HCE43X56V#/260098540_New_Tab_-_Photon_Tiles Here's a punch-list of diffs from the previous design that need addressing: [ ] Updated page background color [ ] Update search field style [ ] Updated gear flyout w/ removed link to SUMO article [ ] Updated font-style on Tile Titles [ ] Updated color for Title Title backgrounds [ ] Updated dropshadow for Tiles [ ] Updated pin and close icons for hover states [ ] Updated gear icon (top-right corner) [ ] Onboarding icon should use the new Firefox Logo

SvG file needed for icon update.

forward icon, in case you can't use the system one.

search icon in case one can't use the system icon.

Disregard the item about removing the link to "Learn about New Tab" from the gear flyout. The link should stay.

Updates: * Search field updates to look like about:home * Font styles on Tile titles * Background colours on Tile titles * Updated drop shadow for Tiles (and removed the blue hover state) * Update all icons (pin-default, pin-hover, pinned, settings gear in top right, search arrow, dismiss tile button)

Spec updated with some changes, but already coordinated with Ursula (updated search field and returned the 'Learn about New Tab' option in the gear flyout).

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198406 Thanks for putting this together, Ursula! Some first thoughts: * if you haven't already it's probably worth using the PerfectPixel addon to make sure that these match the mocks as expected. * another thing that should be possible with PerfectPixel is if you taken a screenshot of the unpatched version of tiles, you should be able to overlay it on top of your changes, and that would allow you to fairly easily identify if there are any unexpected visual changes, at least in the default state. I'd suggest doing this on at least Windows, where the vast majority of the users are, since CSS has lots of global state, and the system themes can interact with other CSS in unexpected ways. I've done a bunch of code inspection, and attached are some comments/questions related to that. I still need to do hand-testing and play around some with DOMi as well, but I'm so tired that I can barely see straight, so I'll have to dig into that in the morning. The r- is just because this is going to need at least one more iteration, and this set of comments should provide a reasonable point to dig in further. ::: browser/base/content/newtab/newTab.css:266 (Diff revision 1) > - height: 36px; > - background: url("chrome://browser/skin/search-indicator-magnifying-glass.svg") center center no-repeat; > position: absolute; > + offset-inline-start: 0; > + height: 35px; > + width: 35px; Given that we're trying to keep the patch as small and readable as possible, I'd strongly suggest preserving the ordering of the properties, as it makes it easier to understand what exactly has changed and doesn't bloat the diff with code motion. I think it's worth fixing this for the entire patch, given the current circumstances. ::: browser/themes/shared/newtab/controls.svg:1 (Diff revision 1) > -<?xml version="1.0" encoding="utf-8"?> > +<?xml version="1.0" encoding="UTF-8"?> The original version of this file appears to be hand-edited (though it was presumably originally generated). The new version has (apparently) not had the hand-editing, and hopefully nothing added in the editing in the old version will effect anything outside the SVG file (eg stuff like .glyphShape-style isn't used in the new version), and one would hope that nothing from the CSS files reaches in and depends on those sorts of things, but I've seen stranger stuff in mozilla code base in the paste. I suspect there's nothing scary here, but I've had reviewers sometimes want to dig into the details of this sort of thing in the past. My SVG-fu is not strong, so I think we probably want someone else to have a look at this part of the patch before all is said and done. Let's talk about this a bit more Thursday. ::: browser/themes/shared/newtab/newTab.inc.css (Diff revision 1) > /* SITES */ > .newtab-site { > border-radius: var(--cell-corner-radius); > - box-shadow: 0 1px 3px #c1c1c1; > + box-shadow: 0 2px 4px #c1c1c1; > text-decoration: none; > - transition-property: top, left, opacity, box-shadow, background-color; Why did the transition-property go away? ::: browser/themes/shared/newtab/newTab.inc.css (Diff revision 1) > - margin: -2px; > -} > - > -.newtab-site[dragged] { > - transition-property: box-shadow, background-color; > - background-color: rgb(242,242,242); Why did the dragged stuff disappear?

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198406 > Given that we're trying to keep the patch as small and readable as possible, I'd strongly suggest preserving the ordering of the properties, as it makes it easier to understand what exactly has changed and doesn't bloat the diff with code motion. > > I think it's worth fixing this for the entire patch, given the current circumstances. I can try, but for the search stuff the css is almost entirely different, so either way the diff will be pretty much the same > The original version of this file appears to be hand-edited (though it was presumably originally generated). > > The new version has (apparently) not had the hand-editing, and hopefully nothing added in the editing in the old version will effect anything outside the SVG file (eg stuff like .glyphShape-style isn't used in the new version), and one would hope that nothing from the CSS files reaches in and depends on those sorts of things, but I've seen stranger stuff in mozilla code base in the paste. > > I suspect there's nothing scary here, but I've had reviewers sometimes want to dig into the details of this sort of thing in the past. My SVG-fu is not strong, so I think we probably want someone else to have a look at this part of the patch before all is said and done. > > Let's talk about this a bit more Thursday. The controls.svg is only ever used in newtab related css, and from what I can see there's nothing particularly unique about the svg that is reflected in the css. The only thing it needs is that the -moz-image-rect lines up with the coorindates, which it does even with the new version > Why did the transition-property go away? Without the change of box-shadow and background color on hover/drag, there are not transition effects anymore. I guess I can leave in the top, left, opacity, but I couldn't visually see a difference with and without those transition-properties. Also in browser/base/content/newtab/newTab.css, the .newtab-cell also has a transition-property: top, left, opacity which would override this one anyways > Why did the dragged stuff disappear? This added a blue box-shadow around the tile both on hover and when you drag the tile and *slightly* modified the background color for some reason, which after speaking with Aaron, was to be removed in the newest specs

(In reply to Ursula Sarracini (:ursula) from comment #9) > Without the change of box-shadow and background color on hover/drag, there > are not transition effects anymore. I guess I can leave in the top, left, > opacity, but I couldn't visually see a difference with and without those > transition-properties. Also in browser/base/content/newtab/newTab.css, the > .newtab-cell also has a transition-property: top, left, opacity which would > override this one anyways > Sorry, .newtab-site I mean, not .newtab-cell

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198596 ::: browser/base/content/newtab/newTab.css:243 (Diff revision 1) > } > > body.compact #newtab-search-container { > margin-top: 0; > margin-bottom: 80px; > } According to comps, font-family should be changed to system ::: browser/base/content/newtab/newTab.css:244 (Diff revision 1) > > body.compact #newtab-search-container { > margin-top: 0; > margin-bottom: 80px; > } > Background colour should be #F9F9FA, it looks like it's #F9F9F9 ::: browser/base/content/newtab/newTab.css:314 (Diff revision 1) > - box-shadow: 0 1px 1px hsla(211,79%,6%,.1) inset, > - 0 0 1px hsla(211,79%,6%,.2) inset; > - transition-duration: 0ms; > } > > /* CUSTOMIZE */ The customize button is the wrong colour (too dark) and too large; it also doesn't have a photon hover state (it should be grey bg, no blue/border) I also noticed that it doesn't seem to be positioned correctly for rtl, although that seems to be an existing issue: https://k88hudson-screenshots.s3.amazonaws.com/screen-shots/Screen_Shot_2017-10-26_at_10.27.58_AM.png Might be worth fixing up quickly if it's easy? ::: browser/themes/shared/newtab/newTab.inc.css:198 (Diff revision 1) > } > > /* TITLES */ > > .newtab-title { > - background-color: #F2F2F2; > + background-color: #F9F9FA; color should be #0C0C0D weight should be semibold ::: browser/themes/shared/newtab/newTab.inc.css:235 (Diff revision 1) > > -.newtab-site[pinned] .newtab-title { > - padding-inline-start: 24px; > -} > - > .newtab-site[pinned] .newtab-title::before { The icons for pinned / close are too large and they wrong colour (they look too light / icons are too close to the edges, take a look at the comp)

Ok looks good to me now, thanks!

Tracking 57+, especially in light of Bug 1411797.

I've checked the patch out on both a windows and a linux machine and everything looks consistent

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198708 ::: browser/base/content/newtab/newTab.css:251 (Diff revision 4) > opacity: 0; > pointer-events: none; > } > > #newtab-search-form { > - display: -moz-box; > + cursor: default; On Mac, at least, the computed style for the old version was "cursor: auto", and all the poking around I've done hasn't yet turned up any changes. The extremely vague documentation I've found for "cursor: default" doesn't make it any way clear what default is really supposed to do, and so this change _may_ be unnecessary (or even undesirable?). I'd suggest testing on Windows, and if there's no visible difference there either, finding someone who has more expertise here to look at this change. ::: browser/base/content/newtab/newTab.css:252 (Diff revision 4) > pointer-events: none; > } > > #newtab-search-form { > - display: -moz-box; > + cursor: default; > + display: flex; I assume this was cribbed from the about:home CSS. It seems to work OK, but it does mean that this is the only thing on this page that uses the standardized CSS flexbox model; everything else on the page uses the XUL -moz-box model. I would suspect this is ok, but I've done almost no work mixing the box models. I'd suggest finding someone who has and asking for their eyes on it. ::: browser/base/content/newtab/newTab.css:266 (Diff revision 4) > - height: 36px; > - background: url("chrome://browser/skin/search-indicator-magnifying-glass.svg") center center no-repeat; > position: absolute; > + offset-inline-start: 0; > + height: 35px; > + width: 35px; Moving these up above background should make the diff smaller & more readable.

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198734 ::: browser/base/content/newtab/newTab.css:274 (Diff revision 4) > - border-color: hsla(210,54%,20%,.15) hsla(210,54%,20%,.17) hsla(210,54%,20%,.2); > - box-shadow: 0 1px 0 hsla(210,65%,9%,.02) inset, > - 0 0 2px hsla(210,65%,9%,.1) inset, > - 0 1px 0 hsla(0,0%,100%,.2); > color: inherit; > - unicode-bidi: plaintext; > + padding: 0; Why did this go away? I'm concerned that tt might regress https://bugzilla.mozilla.org/show_bug.cgi?id=1269186.

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198736

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System https://reviewboard.mozilla.org/r/193074/#review198738 This looks good to me. r=dmose, with the requested property re-ordering. When you're making the list of stuff QA should focus on, definitely include RTL, dragging, and hovering. ::: browser/base/content/newtab/newTab.css:287 (Diff revision 5) > -#newtab-search-text:focus, > -#newtab-search-text[autofocus] { > - border-color: hsla(206,100%,60%,.6) hsla(206,76%,52%,.6) hsla(204,100%,40%,.6); > +#newtab-search-text:focus { > + border-color: #0A84FF; > + box-shadow: 0 0 0 2px #0A84FF; > } > > #newtab-search-submit { Reordering the properties here should make the patch smaller.

Pushed by usarracini@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/352a1e6e471f Update Tiles new tab page to match Photon Design System r=dmose

I have verified that the old about:newtab page has been updated on Windows 10 x64, Windows 7 x64, Mac 10.12.6, Mac 10.11.6, Arch Linux x64 and Ubuntu 16.04 x32 using Nightly 58.0a1 Build ID 20171029220112.

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System Approval Request Comment [Feature/Bug causing the regression]: Old about:newtab designs are out of date [User impact if declined]: Old about:newtab will not match Photon Design System [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes, 20171029220112 [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Not really [Why is the change risky/not risky?]: It's just CSS changes on about:newtab to match Photon designs [String changes made/needed]: None

Comment on attachment 8922079 [details] Bug 1411419 - Update Tiles new tab page to match Photon Design System Sounds like a must fix, Beta57+