Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2750483003:
Harmony - Remove icon from internal page info dialog view in Material Design mode. (Closed)
DescriptionRemove icon from internal page info dialog view in Material Design mode.
This should remove the icon according to the screen shot in the referenced issue.
BUG=652028
Review-Url: https://codereview.chromium.org/2750483003
Cr-Commit-Position: refs/heads/master@{#458054}
Committed: https://chromium.googlesource.com/chromium/src/+/d7ba3aed77fb170397a571512cb60676cd4915a2
Patch Set 1 #Patch Set 2 : Added comment referencing another CL which will affect this code #
Total comments: 1
Patch Set 3 : Use the new ShouldShowWindowIcon() method on LayoutDelegate #Messages
Total messages: 19 (12 generated)
Description was changed from ========== Remove icon from internal page info dialog view in Material Design mode. This should remove the icon according to the screen shot in the referenced issue. BUG=652028 ========== to ========== Remove icon from internal page info dialog view in Material Design mode. This should remove the icon according to the screen shot in the referenced issue. BUG=652028 ==========
kylixrd@chromium.org changed reviewers: + benwells@chromium.org
Material Design work.
The CQ bit was checked by kylixrd@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.
lgtm with a nit https://codereview.chromium.org/2750483003/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc (right): https://codereview.chromium.org/2750483003/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:313: // once CL https://codereview.chromium.org/2746223002 has landed. Nit: it's a bit weird to reference CLs in code, e.g. the CL you're referencing might get closed for reasons. Instead I'd just put a TODO: change to '....'.
On 2017/03/14 23:19:57, benwells wrote: > lgtm with a nit > > https://codereview.chromium.org/2750483003/diff/20001/chrome/browser/ui/views... > File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc > (right): > > https://codereview.chromium.org/2750483003/diff/20001/chrome/browser/ui/views... > chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:313: // > once CL https://codereview.chromium.org/2746223002 has landed. > Nit: it's a bit weird to reference CLs in code, e.g. the CL you're referencing > might get closed for reasons. Instead I'd just put a TODO: change to '....'. I'll hold this CL open until that CL lands and then upload a new patch to this CL with that change. It should land within a day or so.
The CQ bit was checked by kylixrd@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from benwells@chromium.org Link to the patchset: https://codereview.chromium.org/2750483003/#ps40001 (title: "Use the new ShouldShowWindowIcon() method on LayoutDelegate")
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_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_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 kylixrd@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": 40001, "attempt_start_ts": 1490014955729750,
"parent_rev": "8a2cd668f09567fdd1fcd01b4f542d1928a277d4", "commit_rev":
"d7ba3aed77fb170397a571512cb60676cd4915a2"}
Message was sent while issue was closed.
Description was changed from ========== Remove icon from internal page info dialog view in Material Design mode. This should remove the icon according to the screen shot in the referenced issue. BUG=652028 ========== to ========== Remove icon from internal page info dialog view in Material Design mode. This should remove the icon according to the screen shot in the referenced issue. BUG=652028 Review-Url: https://codereview.chromium.org/2750483003 Cr-Commit-Position: refs/heads/master@{#458054} Committed: https://chromium.googlesource.com/chromium/src/+/d7ba3aed77fb170397a571512cb6... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/d7ba3aed77fb170397a571512cb6... |
