Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2966163004:
[cr-action-menu] Use clientWidth for rtl flipping. (Closed)
|
Created:
3 years, 5 months ago by calamity Modified:
3 years, 5 months ago Reviewers:
dpapad CC:
chrome-apps-syd-reviews_chromium.org, chromium-reviews, michaelpg+watch-elements_chromium.org, oshima+watch_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[cr-action-menu] Use clientWidth for rtl flipping.
This CL fixes a bug where the action menu would use the entire body's
length as the viewport when flipping coordinates in RTL.
It also moves the scroll position saving into showAtPosition so that
both show methods account for scroll correctly.
BUG=734984
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2966163004
Cr-Original-Commit-Position: refs/heads/master@{#485181}
Committed: https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56d84ef504a232
Review-Url: https://codereview.chromium.org/2966163004
Cr-Commit-Position: refs/heads/master@{#485578}
Committed: https://chromium.googlesource.com/chromium/src/+/30fc850ad1f90068d6dcb4733870aa4ebc94cdc6
Patch Set 1 : #
Total comments: 2
Patch Set 2 : clean up tests #Patch Set 3 : make test use integer values #Patch Set 4 : fix test on mac #
Messages
Total messages: 40 (32 generated)
Description was changed from ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. BUG=734984 ========== to ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by calamity@chromium.org to run a CQ dry run
Patchset #2 (id:20001) has been deleted
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
calamity@chromium.org changed reviewers: + dpapad@chromium.org
PTAL, thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM with nit. https://codereview.chromium.org/2966163004/diff/40001/chrome/test/data/webui/... File chrome/test/data/webui/cr_elements/cr_action_menu_test.js (right): https://codereview.chromium.org/2966163004/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/cr_elements/cr_action_menu_test.js:362: document.body.scrollLeft = 500; The "offscreen scroll positioning" test is becoming a very large test. Can we refactor it to more targeted smaller tests (for example separate tests for RTL and one for LTR)?
https://codereview.chromium.org/2966163004/diff/40001/chrome/test/data/webui/... File chrome/test/data/webui/cr_elements/cr_action_menu_test.js (right): https://codereview.chromium.org/2966163004/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/cr_elements/cr_action_menu_test.js:362: document.body.scrollLeft = 500; On 2017/07/06 22:05:37, dpapad wrote: > The "offscreen scroll positioning" test is becoming a very large test. Can we > refactor it to more targeted smaller tests (for example separate tests for RTL > and one for LTR)? Done.
The CQ bit was checked by calamity@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: This issue passed the CQ dry run.
The CQ bit was checked by calamity@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org Link to the patchset: https://codereview.chromium.org/2966163004/#ps60001 (title: "clean up tests")
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": 1499654141345260,
"parent_rev": "9106b65ff7e9eab437325d292b2dad46d29c7cb8", "commit_rev":
"87be8f4c1eaef26b7c6d7ddb2c56d84ef504a232"}
Message was sent while issue was closed.
Description was changed from ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2966163004 Cr-Commit-Position: refs/heads/master@{#485181} Committed: https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:60001) has been created in https://codereview.chromium.org/2980463002/ by calamity@chromium.org. The reason for reverting is: Breaks on https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.11%20Tests....
Message was sent while issue was closed.
Description was changed from ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2966163004 Cr-Commit-Position: refs/heads/master@{#485181} Committed: https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56... ========== to ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2966163004 Cr-Commit-Position: refs/heads/master@{#485181} Committed: https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56... ==========
The CQ bit was checked by calamity@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: This issue passed the CQ dry run.
The CQ bit was checked by calamity@chromium.org to run a CQ dry run
The CQ bit was unchecked by calamity@chromium.org
The CQ bit was checked by calamity@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org Link to the patchset: https://codereview.chromium.org/2966163004/#ps100001 (title: "fix test on mac")
The CQ bit was unchecked by calamity@chromium.org
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 calamity@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org Link to the patchset: https://codereview.chromium.org/2966163004/#ps100001 (title: "fix test on mac")
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": 100001, "attempt_start_ts": 1499760835902600,
"parent_rev": "358df94ebc08067e0005fbdf6f34f2fe79cd7513", "commit_rev":
"30fc850ad1f90068d6dcb4733870aa4ebc94cdc6"}
Message was sent while issue was closed.
Description was changed from ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2966163004 Cr-Commit-Position: refs/heads/master@{#485181} Committed: https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56... ========== to ========== [cr-action-menu] Use clientWidth for rtl flipping. This CL fixes a bug where the action menu would use the entire body's length as the viewport when flipping coordinates in RTL. It also moves the scroll position saving into showAtPosition so that both show methods account for scroll correctly. BUG=734984 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2966163004 Cr-Original-Commit-Position: refs/heads/master@{#485181} Committed: https://chromium.googlesource.com/chromium/src/+/87be8f4c1eaef26b7c6d7ddb2c56... Review-Url: https://codereview.chromium.org/2966163004 Cr-Commit-Position: refs/heads/master@{#485578} Committed: https://chromium.googlesource.com/chromium/src/+/30fc850ad1f90068d6dcb4733870... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001) as https://chromium.googlesource.com/chromium/src/+/30fc850ad1f90068d6dcb4733870... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
