MD History: Shift focus to overflow menu icon while removing bookmark

MD History: Shift focus to overflow menu icon while removing bookmark

After clicking on the bookmark star to remove bookmark of an url, the focus
highlight is shifted to overflow menu icon which is on the right of the star.

BUG=625567
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/b3ec3feb3034ef9b28c320b27696094bbab654b3
Cr-Commit-Position: refs/heads/master@{#406426}

[lshang, 2016-07-11 07:17:22]

Description was changed from

==========
MD History: Shift focus to overflow menu icon while removing bookmark


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


change


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


try

BUG=
==========

to

==========
MD History: Shift focus to overflow menu icon while removing bookmark


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


change


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


try

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[lshang, 2016-07-15 03:32:26]

Description was changed from

==========
MD History: Shift focus to overflow menu icon while removing bookmark


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


change


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


try

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Shift focus to overflow menu icon while removing bookmark


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


change


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


try

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

[lshang, 2016-07-15 03:41:43]

The CQ bit was checked by lshang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 03:42:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 03:42:01]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[lshang, 2016-07-15 04:00:24]

Description was changed from

==========
MD History: Shift focus to overflow menu icon while removing bookmark


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


change


Merge branch 'master' into MDH_move_focus_after_remove_bookmark


try

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Shift focus to overflow menu icon while removing bookmark

After clicking on the bookmark star to remove bookmark of an url, the focus
highlight is shifted to overflow menu icon which is on the right of the star.

BUG=625567
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

[lshang, 2016-07-15 04:00:36]

Patchset #2 (id:20001) has been deleted

[lshang, 2016-07-15 04:05:25]

Patchset #1 (id:1) has been deleted

[lshang, 2016-07-15 04:07:16]

lshang@chromium.org changed reviewers:
+ tsergeant@chromium.org

[lshang, 2016-07-15 04:07:17]

PTAL thanks! \(≥▽≤)/

[commit-bot: I haz the power, 2016-07-15 04:47:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 04:47:29]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2016-07-15 04:50:00]

tsergeant@chromium.org changed reviewers:
+ calamity@chromium.org

[tsergeant, 2016-07-15 04:50:01]

+calamity for a question below.

https://codereview.chromium.org/2141433003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_item.js (right):

https://codereview.chromium.org/2141433003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history_item.js:53: if
(this.$['bookmark-star'] == this.root.activeElement) {
Nit: Remove {}

https://codereview.chromium.org/2141433003/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/history_item_test.js (right):

https://codereview.chromium.org/2141433003/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/history_item_test.js:95: assertEquals(
I'm not certain whether we want to test these values. In theory, checking the
value of `starred` should be enough, rather than relying on the CSS as well.

calamity: What do you think?

[calamity, 2016-07-18 05:03:43]

https://codereview.chromium.org/2141433003/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/history_item_test.js (right):

https://codereview.chromium.org/2141433003/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/history_item_test.js:95: assertEquals(
On 2016/07/15 04:50:01, tsergeant wrote:
> I'm not certain whether we want to test these values. In theory, checking the
> value of `starred` should be enough, rather than relying on the CSS as well.
> 
> calamity: What do you think?

I'm happy to leave this here. I feel like this is one of those things that could
slip through the cracks otherwise (where the UI doesn't reflect the model).

[tsergeant, 2016-07-19 03:36:06]

https://codereview.chromium.org/2141433003/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/history_item_test.js (right):

https://codereview.chromium.org/2141433003/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/history_item_test.js:95: assertEquals(
On 2016/07/18 05:03:43, calamity wrote:
> On 2016/07/15 04:50:01, tsergeant wrote:
> > I'm not certain whether we want to test these values. In theory, checking
the
> > value of `starred` should be enough, rather than relying on the CSS as well.
> > 
> > calamity: What do you think?
> 
> I'm happy to leave this here. I feel like this is one of those things that
could
> slip through the cracks otherwise (where the UI doesn't reflect the model).

sure, sgtm

[lshang, 2016-07-19 07:52:21]

The CQ bit was checked by lshang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 07:52:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 07:52:53]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[lshang, 2016-07-19 07:55:10]

Thanks all!

and tsergeant@, 'sgtm' seems won't change it to bright shiny green (⊙０⊙)

https://codereview.chromium.org/2141433003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_item.js (right):

https://codereview.chromium.org/2141433003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history_item.js:53: if
(this.$['bookmark-star'] == this.root.activeElement) {
On 2016/07/15 04:50:01, tsergeant wrote:
> Nit: Remove {}

Done.

[tsergeant, 2016-07-19 08:27:50]

shiny green lgtm

[commit-bot: I haz the power, 2016-07-19 10:28:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-19 10:28:19]

Dry run: This issue passed the CQ dry run.

[lshang, 2016-07-20 00:12:06]

The CQ bit was checked by lshang@chromium.org

[commit-bot: I haz the power, 2016-07-20 00:13:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 00:13:35]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-20 00:20:35]

Description was changed from

==========
MD History: Shift focus to overflow menu icon while removing bookmark

After clicking on the bookmark star to remove bookmark of an url, the focus
highlight is shifted to overflow menu icon which is on the right of the star.

BUG=625567
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Shift focus to overflow menu icon while removing bookmark

After clicking on the bookmark star to remove bookmark of an url, the focus
highlight is shifted to overflow menu icon which is on the right of the star.

BUG=625567
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-20 00:20:37]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-07-20 00:20:43]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-20 00:22:05]

Description was changed from

==========
MD History: Shift focus to overflow menu icon while removing bookmark

After clicking on the bookmark star to remove bookmark of an url, the focus
highlight is shifted to overflow menu icon which is on the right of the star.

BUG=625567
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Shift focus to overflow menu icon while removing bookmark

After clicking on the bookmark star to remove bookmark of an url, the focus
highlight is shifted to overflow menu icon which is on the right of the star.

BUG=625567
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/b3ec3feb3034ef9b28c320b27696094bbab654b3
Cr-Commit-Position: refs/heads/master@{#406426}
==========

[commit-bot: I haz the power, 2016-07-20 00:22:06]

Patchset 2 (id:??) landed as
https://crrev.com/b3ec3feb3034ef9b28c320b27696094bbab654b3
Cr-Commit-Position: refs/heads/master@{#406426}