Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2137083004:
Fix overlapping of shelf options on supervised user creation page when rotating screen (Closed)
|
Created:
4 years, 5 months ago by Qiang(Joe) Xu Modified:
4 years, 5 months ago Reviewers:
stevenjb CC:
chromium-reviews, kalyank, sadrul Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix overlapping of shelf options on supervised user creation page when rotating screen
BUG=627040
TEST=manual test saw bug fixed
Committed: https://crrev.com/bdf7241b6babfab7e3c1970c508ba43a306a3b47
Cr-Commit-Position: refs/heads/master@{#405045}
Patch Set 1 #Patch Set 2 : check primary display shelf exists before call alignment() #
Total comments: 2
Patch Set 3 : revert if #Messages
Total messages: 29 (19 generated)
The CQ bit was checked by warx@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: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by warx@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...
Description was changed from ========== Fix overlapping of shelf options on supervised user creation page when rotating screen BUG=627040 TEST=manual test saw bug fixed ========== to ========== Fix overlapping of shelf options on supervised user creation page when rotating screen BUG=627040 TEST=manual test saw bug fixed ==========
warx@chromium.org changed reviewers: + stevenjb@chromium.org
Hi Steven, could you please take a look? tks
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2137083004/diff/20001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc (right): https://codereview.chromium.org/2137083004/diff/20001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc:1677: return; nit: Normally we prefer early exit like this, however in this case since this is a Changed() handler it might be better to invert the if and move SetShelfBehaviorsFromPrefs() to the if body, that way if additional code gets added to this handler it will still get called. (That said, anybody adding code should notice and make the change, so this is also fine as-is).
https://codereview.chromium.org/2137083004/diff/20001/chrome/browser/ui/ash/l... File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc (right): https://codereview.chromium.org/2137083004/diff/20001/chrome/browser/ui/ash/l... chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc:1677: return; On 2016/07/11 22:04:24, stevenjb wrote: > nit: Normally we prefer early exit like this, however in this case since this is > a Changed() handler it might be better to invert the if and move > SetShelfBehaviorsFromPrefs() to the if body, that way if additional code gets > added to this handler it will still get called. (That said, anybody adding code > should notice and make the change, so this is also fine as-is). done. tks for this tip.
The CQ bit was checked by warx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stevenjb@chromium.org Link to the patchset: https://codereview.chromium.org/2137083004/#ps40001 (title: "revert if")
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
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by warx@chromium.org
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 warx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stevenjb@chromium.org Link to the patchset: https://codereview.chromium.org/2137083004/#ps60001 (title: "revert if")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix overlapping of shelf options on supervised user creation page when rotating screen BUG=627040 TEST=manual test saw bug fixed ========== to ========== Fix overlapping of shelf options on supervised user creation page when rotating screen BUG=627040 TEST=manual test saw bug fixed ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Fix overlapping of shelf options on supervised user creation page when rotating screen BUG=627040 TEST=manual test saw bug fixed ========== to ========== Fix overlapping of shelf options on supervised user creation page when rotating screen BUG=627040 TEST=manual test saw bug fixed Committed: https://crrev.com/bdf7241b6babfab7e3c1970c508ba43a306a3b47 Cr-Commit-Position: refs/heads/master@{#405045} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/bdf7241b6babfab7e3c1970c508ba43a306a3b47 Cr-Commit-Position: refs/heads/master@{#405045} |
