|
|
Chromium Code Reviews|
Created:
4 years, 7 months ago by dschuyler Modified:
4 years, 7 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 Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[MD settings] restore side nav drop icons
This CL fixes an issue introduced by CL 1943133002
which is that the drop (triangle) icons on the
side nav disappeared.
BUG=608989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/d474f79dfe0b8772e60326f6d20c7df48110f296
Cr-Commit-Position: refs/heads/master@{#391939}
Patch Set 1 #Patch Set 2 : adding test #
Total comments: 4
Patch Set 3 : nits #
Messages
Total messages: 20 (9 generated)
Description was changed from ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 194313302 which is that the drop (triangle) icons on the side nav disappeared. BUG=584478 ========== to ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 194313302 which is that the drop (triangle) icons on the side nav disappeared. BUG=584478 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 194313302 which is that the drop (triangle) icons on the side nav disappeared. BUG=584478 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 194313302 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
dschuyler@chromium.org changed reviewers: + dpapad@chromium.org
dschuyler@: do you mean this CL? https://codereview.chromium.org/1943133002/
On 2016/05/04 at 01:33:20, dschuyler wrote: > Can we add some tests for settings-menu element? I asked about this at https://codereview.chromium.org/1943133002/#msg4, and did not realize it was not addressed before LGing.
Description was changed from ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 194313302 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 1943133002 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
On 2016/05/04 01:41:58, Dan Beam wrote: > dschuyler@: do you mean this CL? https://codereview.chromium.org/1943133002/ fixed CL number in description
added test
LGTM https://codereview.chromium.org/1949913002/diff/20001/chrome/test/data/webui/... File chrome/test/data/webui/settings/settings_menu_test.js (right): https://codereview.chromium.org/1949913002/diff/20001/chrome/test/data/webui/... chrome/test/data/webui/settings/settings_menu_test.js:27: '.menu-trigger iron-icon'); Nit: Let's assert that the element was actually found before making further assertions. assertTrue(!!ironIconElement); https://codereview.chromium.org/1949913002/diff/20001/chrome/test/data/webui/... chrome/test/data/webui/settings/settings_menu_test.js:32: page: 'basic', section: 'reset', subpage: [] Nit: basic page does not have a 'reset' section, maybe replace 'reset' with an existing section?
Patchset #3 (id:40001) has been deleted
https://codereview.chromium.org/1949913002/diff/20001/chrome/test/data/webui/... File chrome/test/data/webui/settings/settings_menu_test.js (right): https://codereview.chromium.org/1949913002/diff/20001/chrome/test/data/webui/... chrome/test/data/webui/settings/settings_menu_test.js:27: '.menu-trigger iron-icon'); On 2016/05/05 21:13:48, dpapad wrote: > Nit: Let's assert that the element was actually found before making further > assertions. > assertTrue(!!ironIconElement); Done. https://codereview.chromium.org/1949913002/diff/20001/chrome/test/data/webui/... chrome/test/data/webui/settings/settings_menu_test.js:32: page: 'basic', section: 'reset', subpage: [] On 2016/05/05 21:13:48, dpapad wrote: > Nit: basic page does not have a 'reset' section, maybe replace 'reset' with an > existing section? Done.
The CQ bit was checked by dschuyler@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/1949913002/#ps60001 (title: "nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1949913002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1949913002/60001
Message was sent while issue was closed.
Description was changed from ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 1943133002 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 1943133002 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 1943133002 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [MD settings] restore side nav drop icons This CL fixes an issue introduced by CL 1943133002 which is that the drop (triangle) icons on the side nav disappeared. BUG=608989 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/d474f79dfe0b8772e60326f6d20c7df48110f296 Cr-Commit-Position: refs/heads/master@{#391939} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d474f79dfe0b8772e60326f6d20c7df48110f296 Cr-Commit-Position: refs/heads/master@{#391939} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
