Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2968483002:
Improve the "Learn more" link on the Incognito New Tab page. (Closed)
|
Created:
3 years, 5 months ago by msramek Modified:
3 years, 5 months ago Reviewers:
Michael van Ouwerkerk CC:
chromium-reviews, noyau+watch_chromium.org, ntp-dev+reviews_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionImprove the "Learn more" link on the Incognito New Tab page.
The "Learn more" link is a standalone element on a narrow (<= 720dp)
screen, but is a part of the subtitle on a wide (> 720dp) screen.
This is achieved by showing/hiding the standalone element and
including/removing a ClickableSpan in the subtitle.
Currently, none of the "Learn more" links has any visual response to
clicking (e.g. highlight), which is consistent with other such
links in Chrome's native UI. However, it could be added in the future.
BUG=693525
Review-Url: https://codereview.chromium.org/2968483002
Cr-Commit-Position: refs/heads/master@{#483722}
Committed: https://chromium.googlesource.com/chromium/src/+/9828df63fe8ba13923c6ee9133fd7584b7dd3fc0
Patch Set 1 #Patch Set 2 : Rebase. #
Total comments: 4
Patch Set 3 : Addressed nits #Patch Set 4 : Rebase. #
Depends on Patchset: Messages
Total messages: 31 (26 generated)
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
msramek@chromium.org changed reviewers: + mvanouwerkerk@google.com
Hi again Michael, After you review https://codereview.chromium.org/2963523002/, please also have a look at this one :) It's based on top of it. Thanks! Martin
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Improve the "Learn more" link on the Incognito New Tab page. The "Learn more" link is a standalone element on a narrow (<= 720dp) screen, but is a part of the subtitle on a wide (> 720dp) screen. This is achieved by showing/hiding the standalone element and including/removing a ClickableSpan in the subtitle. Currently, none of the "Learn more" links has any visual response to clicking (e.g. highlight), which is consistent with other such links in Chrome's native UI. However, it could be added in the future. BUG=693525 ========== to ========== Improve the "Learn more" link on the Incognito New Tab page. The "Learn more" link is a standalone element on a narrow (<= 720dp) screen, but is a part of the subtitle on a wide (> 720dp) screen. This is achieved by showing/hiding the standalone element and including/removing a ClickableSpan in the subtitle. Currently, none of the "Learn more" links has any visual response to clicking (e.g. highlight), which is consistent with other such links in Chrome's native UI. However, it could be added in the future. BUG=693525 ==========
mvanouwerkerk@chromium.org changed reviewers: + mvanouwerkerk@chromium.org - mvanouwerkerk@google.com
lgtm with nits https://codereview.chromium.org/2968483002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java (right): https://codereview.chromium.org/2968483002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java:70: private @ColorRes int mColor; nit: final https://codereview.chromium.org/2968483002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java:71: private IncognitoNewTabPageManager mManager; nit: final
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks again! https://codereview.chromium.org/2968483002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java (right): https://codereview.chromium.org/2968483002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java:70: private @ColorRes int mColor; On 2017/06/30 10:35:38, Michael van Ouwerkerk wrote: > nit: final Done. https://codereview.chromium.org/2968483002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java:71: private IncognitoNewTabPageManager mManager; On 2017/06/30 10:35:38, Michael van Ouwerkerk wrote: > nit: final Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by msramek@chromium.org
The CQ bit was checked by msramek@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mvanouwerkerk@chromium.org Link to the patchset: https://codereview.chromium.org/2968483002/#ps60001 (title: "Rebase.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498835267839410,
"parent_rev": "e2e6ca94f4b015a822303a88ee502c7103aa0766", "commit_rev":
"9828df63fe8ba13923c6ee9133fd7584b7dd3fc0"}
Message was sent while issue was closed.
Description was changed from ========== Improve the "Learn more" link on the Incognito New Tab page. The "Learn more" link is a standalone element on a narrow (<= 720dp) screen, but is a part of the subtitle on a wide (> 720dp) screen. This is achieved by showing/hiding the standalone element and including/removing a ClickableSpan in the subtitle. Currently, none of the "Learn more" links has any visual response to clicking (e.g. highlight), which is consistent with other such links in Chrome's native UI. However, it could be added in the future. BUG=693525 ========== to ========== Improve the "Learn more" link on the Incognito New Tab page. The "Learn more" link is a standalone element on a narrow (<= 720dp) screen, but is a part of the subtitle on a wide (> 720dp) screen. This is achieved by showing/hiding the standalone element and including/removing a ClickableSpan in the subtitle. Currently, none of the "Learn more" links has any visual response to clicking (e.g. highlight), which is consistent with other such links in Chrome's native UI. However, it could be added in the future. BUG=693525 Review-Url: https://codereview.chromium.org/2968483002 Cr-Commit-Position: refs/heads/master@{#483722} Committed: https://chromium.googlesource.com/chromium/src/+/9828df63fe8ba13923c6ee9133fd... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/9828df63fe8ba13923c6ee9133fd... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
