[Dark mode] Support dark mode on the NTP |
|||||||||||
Issue descriptionOverall tracking for dark mode support on the NTP. Discussion of what's required is in https://docs.google.com/document/d/1PjgxBWgSGsnsoRWufN7GL4XGwwXXAPeq27blotoSEP8/edit ⛆ |
|
|
,
Jan 8
Namrata / Joel - can you provide a bit more detail on the elements called out in the linked doc? Thanks!
,
Jan 9
,
Jan 9
,
Jan 10
,
Jan 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d6ffd96fca2ea364fea8cece7516ef94750805a commit 9d6ffd96fca2ea364fea8cece7516ef94750805a Author: Kristi Park <kristipark@chromium.org> Date: Mon Jan 14 18:58:22 2019 [NTP] Set up dark mode config, and apply dark mode background when enabled Set up the dark mode config to be used in all NTP components. Also apply a dark background for the default NTP when dark mode is enabled. Default NTP: https://screenshot.googleplex.com/DtOZNv99hfd.png Third-party NTP: https://screenshot.googleplex.com/kOcDzLmwTmj.png Bug: 919961 Change-Id: I253cb6655493a5497dcba1fcf16bec4a4e4d19bc Reviewed-on: https://chromium-review.googlesource.com/c/1407951 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Kyle Milka <kmilka@chromium.org> Cr-Commit-Position: refs/heads/master@{#622540} [modify] https://crrev.com/9d6ffd96fca2ea364fea8cece7516ef94750805a/chrome/browser/resources/local_ntp/local_ntp.js [modify] https://crrev.com/9d6ffd96fca2ea364fea8cece7516ef94750805a/chrome/browser/search/local_ntp_source.cc
,
Jan 14
Update on my side. Just getting up to speed and talking to namratakannan@ about NTP dark mode spec. Ill be helping and feel free to ping me if needed.
,
Jan 14
,
Jan 15
I noticed two issues in latest Canary Version 73.0.3672.0 starting with --force-dark-mode: - The Dark New Tab Page flashes white when it opens. - Not sure if only macOS-related, but if rubber band scrolling happens on bottom of the Dark NTP, there appears a white area which doesn't disappears. Looks like the NTP doesn't bounce back to its normal position after the rubber banding animation ends. Would be also very nice, if the rubber banding background could be dark, too. Please let me know, if you need more information or if I should file separate reports for the issues. Thanks :)
,
Jan 15
Currently investigating the white flash. It seems like this has been a recurring issue for a number of years and is a bit complicated to tackle. Fix for the rubberband scrolling is in review.
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a51840f2600ac4de036575c0998a8491b28958e commit 0a51840f2600ac4de036575c0998a8491b28958e Author: Kristi Park <kristipark@chromium.org> Date: Wed Jan 16 04:27:22 2019 [NTP] Fix white flash when opening new tab in dark mode The default NTP background color is excluded from the dark mode spec, which causes a white flash when a new tab is opened in dark mode. Now that the NTP is being updated to support dark mode, this exception is no longer required. Bug: 919961 Change-Id: Ic545ae2a41bcea7aec1f764cafd78aaa1d749841 Reviewed-on: https://chromium-review.googlesource.com/c/1413240 Commit-Queue: Kristi Park <kristipark@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#623101} [modify] https://crrev.com/0a51840f2600ac4de036575c0998a8491b28958e/chrome/browser/themes/theme_properties.cc
,
Jan 16
(6 days ago)
Hey kristipark@, thanks for fixing the white flashing issue. Your CL is not in latest Canary yet, but I checked latest Chromium Snapshot Build Revision 623233, and it seems that your latest CL, that fixes the flash, has also fixed the rubber band scrolling issue. The rubber band background is now dark too (same color like the NTP) and no more white area at the bottom is to see - at least on macOS :) I can retest it when it is in Canary.
,
Jan 16
(6 days ago)
Prompted by issue 922574: the detection logic isn't quite right here. It keeps the NTP dark when the user switches to light mode with `--enable-features=DarkMode` and will fail in general when we remove the feature. I'm not sure how this is architected, but if it's possible to call into NativeTheme, replacing this check https://cs.chromium.org/chromium/src/chrome/browser/search/local_ntp_source.cc?q=local_ntp_source&sq=package:chromium&dr=C&l=572 with ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeEnabled() should fix it
,
Jan 16
(6 days ago)
Now using SystemDarkModeEnabled as of https://crrev.com/c/1407204.
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/737f310470c79fda62d36b7fea0b7f654b1fa50a commit 737f310470c79fda62d36b7fea0b7f654b1fa50a Author: Kristi Park <kristipark@chromium.org> Date: Fri Jan 18 18:55:42 2019 [NTP] Support dark mode for toast notifications This includes custom link notifications, error notifications, promo, and the third-party NTP blacklist notification. Custom link: https://screenshot.googleplex.com/6Y2AEjEDMbL.png Error: https://screenshot.googleplex.com/wqxj7B6BF5S.png Promo: https://screenshot.googleplex.com/MRQsvhTnnUe.png Third-party NTP: https://screenshot.googleplex.com/CzFrGZgYZ94.png Bug: 919961 Change-Id: I86420e3eaa18600c84e88d088529f940b3b535cf Reviewed-on: https://chromium-review.googlesource.com/c/1419205 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Kyle Milka <kmilka@chromium.org> Cr-Commit-Position: refs/heads/master@{#624217} [modify] https://crrev.com/737f310470c79fda62d36b7fea0b7f654b1fa50a/chrome/browser/resources/local_ntp/local_ntp.css [modify] https://crrev.com/737f310470c79fda62d36b7fea0b7f654b1fa50a/chrome/browser/resources/local_ntp/local_ntp.js
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/680ed8f316f5f4241b9a614043d88fa4f3ed8d83 commit 680ed8f316f5f4241b9a614043d88fa4f3ed8d83 Author: Kristi Park <kristipark@chromium.org> Date: Fri Jan 18 20:09:39 2019 [NTP] Use white One Google Bar and white Google logo when in dark mode If dark mode is enabled, use the white One Google Bar and white Google logo. If a theme is installed, use the default theme values. Bug: 919961, 918582 Change-Id: Id9fe74511a7f18e1256372ed3b782fd5266ca543 Reviewed-on: https://chromium-review.googlesource.com/c/1419362 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Kyle Milka <kmilka@chromium.org> Cr-Commit-Position: refs/heads/master@{#624252} [modify] https://crrev.com/680ed8f316f5f4241b9a614043d88fa4f3ed8d83/chrome/browser/resources/local_ntp/local_ntp.js
,
Jan 18
(4 days ago)
,
Today
(18 hours ago)
The NextAction date has arrived: 2019-01-22
,
Today
(9 hours ago)
|
||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by ramyan@chromium.org
, Jan 8