|
|
Created:
3 years, 9 months ago by dschuyler Modified:
3 years, 9 months ago Reviewers:
dpapad CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-settings_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[MD settings] hide extra horizontal scrollbar
This CL prevents an unwanted horizontal scrollbar from flashing at the
bottom of the page when transitioning from a sub-page back to the basic
page.
BUG=645346
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2753943003
Cr-Commit-Position: refs/heads/master@{#457570}
Committed: https://chromium.googlesource.com/chromium/src/+/bfa14e6ef0b8adc0f56d899f3776fe4b960b8978
Patch Set 1 : comment #
Total comments: 3
Messages
Total messages: 19 (12 generated)
Description was changed from ========== [MD settings] hide extra horizontal scrollbar This CL prevents an unwanted horizontal scrollbar from flashing at the bottom of the page when transitioning from a sub-page back to the basic page. BUG=645346 ========== to ========== [MD settings] hide extra horizontal scrollbar This CL prevents an unwanted horizontal scrollbar from flashing at the bottom of the page when transitioning from a sub-page back to the basic page. BUG=645346 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by dschuyler@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 checked by dschuyler@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...
Patchset #1 (id:1) has been deleted
dschuyler@chromium.org changed reviewers: + dpapad@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2753943003/diff/20001/chrome/browser/resource... File chrome/browser/resources/settings/basic_page/basic_page.html (right): https://codereview.chromium.org/2753943003/diff/20001/chrome/browser/resource... chrome/browser/resources/settings/basic_page/basic_page.html:42: overflow: hidden; Does this create a problem when the page is too narrow? Ideally the user should still be able to scroll left and right. Perhaps it would be better to first understand why the scrollbar flashes on the screen during the animation and possibly fix the root cause, instead of just working around it?
https://codereview.chromium.org/2753943003/diff/20001/chrome/browser/resource... File chrome/browser/resources/settings/basic_page/basic_page.html (right): https://codereview.chromium.org/2753943003/diff/20001/chrome/browser/resource... chrome/browser/resources/settings/basic_page/basic_page.html:42: overflow: hidden; On 2017/03/16 20:51:19, dpapad wrote: > Does this create a problem when the page is too narrow? Ideally the user should > still be able to scroll left and right. The content is still fully accessible. > Perhaps it would be better to first understand why the scrollbar flashes on the > screen during the animation and possibly fix the root cause, instead of just > working around it? Can you be more specific about what you feel is incorrect?
LGTM https://codereview.chromium.org/2753943003/diff/20001/chrome/browser/resource... File chrome/browser/resources/settings/basic_page/basic_page.html (right): https://codereview.chromium.org/2753943003/diff/20001/chrome/browser/resource... chrome/browser/resources/settings/basic_page/basic_page.html:42: overflow: hidden; On 2017/03/16 at 21:01:43, dschuyler wrote: > On 2017/03/16 20:51:19, dpapad wrote: > > Does this create a problem when the page is too narrow? Ideally the user should > > still be able to scroll left and right. > > The content is still fully accessible. > > > > Perhaps it would be better to first understand why the scrollbar flashes on the > > screen during the animation and possibly fix the root cause, instead of just > > working around it? > > Can you be more specific about what you feel is incorrect? This suggestion was dependent on the question above. Since it works as expected, it is not applicable. I should have made my comment more clear.
The CQ bit was checked by dschuyler@chromium.org
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": 20001, "attempt_start_ts": 1489700273548530, "parent_rev": "2ef26ee13e1d61e0d52daf64bd4ad2ef7a7907d5", "commit_rev": "bfa14e6ef0b8adc0f56d899f3776fe4b960b8978"}
Message was sent while issue was closed.
Description was changed from ========== [MD settings] hide extra horizontal scrollbar This CL prevents an unwanted horizontal scrollbar from flashing at the bottom of the page when transitioning from a sub-page back to the basic page. BUG=645346 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [MD settings] hide extra horizontal scrollbar This CL prevents an unwanted horizontal scrollbar from flashing at the bottom of the page when transitioning from a sub-page back to the basic page. BUG=645346 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2753943003 Cr-Commit-Position: refs/heads/master@{#457570} Committed: https://chromium.googlesource.com/chromium/src/+/bfa14e6ef0b8adc0f56d899f3776... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:20001) as https://chromium.googlesource.com/chromium/src/+/bfa14e6ef0b8adc0f56d899f3776...
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:20001) has been created in https://codereview.chromium.org/2756253002/ by dbeam@chromium.org. The reason for reverting is: The bug this is fixing https://bugs.chromium.org/p/chromium/issues/detail?id=645346 Is not as bad as the bug it introduced https://bugs.chromium.org/p/chromium/issues/detail?id=702496. |