MD Bookmarks: Implement item selection in new data flow system

MD Bookmarks: Implement item selection in new data flow system

MD Bookmarks has switched to a new data flow system which makes it
easier to bind and modify backend data from the UI. This CL implements
item selection in this new system, making it possible to select
individual items and ctrl/shift select groups of items.

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

Review-Url: https://codereview.chromium.org/2741393002
Cr-Commit-Position: refs/heads/master@{#456996}
Committed: https://chromium.googlesource.com/chromium/src/+/f32efd59c35df91f5f63866666702c139afcac4b

[tsergeant, 2017-03-13 02:08:15]

Description was changed from

==========
MD Bookmarks: Implement item selection in new data flow system

MD Bookmarks has switched to a new data flow system which makes it
easier to bind and modify backend data from the UI. This CL implements
item selection in this new system, making it possible to select
individual items and ctrl/shift select groups of items.

BUG=697706
==========

to

==========
MD Bookmarks: Implement item selection in new data flow system

MD Bookmarks has switched to a new data flow system which makes it
easier to bind and modify backend data from the UI. This CL implements
item selection in this new system, making it possible to select
individual items and ctrl/shift select groups of items.

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

[tsergeant, 2017-03-13 02:21:07]

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

[commit-bot: I haz the power, 2017-03-13 02:21:20]

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

[tsergeant, 2017-03-13 02:21:23]

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

[tsergeant, 2017-03-13 02:21:23]

And here's the follow-up

[commit-bot: I haz the power, 2017-03-13 03:26:35]

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

[commit-bot: I haz the power, 2017-03-13 03:26:36]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-03-14 03:01:45]

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/actions.js (right):

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/actions.js:84: 
Add a TODO to deselect already selected items when control clicked. (Or just
implement it)

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/reducers.js (right):

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:21: var newState = {};
nit: newItems

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:41: count: 0,
Is count used anywhere? Or is it just like, a #define for 0 =P

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:55: case 'finish-search':
Also clear-search.

+Test

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:58: return
SelectionState.selectItems(selection, action);
What happens to removed anchor indexes? They'll explode on the next shift
select, right?

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:270:
ClosedFolderState.updateClosedFolders = function(state, action, nodes) {
Change state to closedFolders to be consistent with the other functions.

https://codereview.chromium.org/2741393002/diff/1/chrome/test/data/webui/md_b...
File chrome/test/data/webui/md_bookmarks/reducers_test.js (right):

https://codereview.chromium.org/2741393002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/reducers_test.js:21: items: 1,
This is supposed to be an Object.

https://codereview.chromium.org/2741393002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/reducers_test.js:30: nextState =
bookmarks.SelectionState.updateSelection(initialState, action);
nextState => state as in previous patch.

[tsergeant, 2017-03-14 05:40:30]

Patchset #2 (id:20001) has been deleted

[tsergeant, 2017-03-14 05:42:02]

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/actions.js (right):

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/actions.js:84: 
On 2017/03/14 03:01:45, calamity wrote:
> Add a TODO to deselect already selected items when control clicked. (Or just
> implement it)

TODO-ed since we'll want to be careful about how this interacts with
shift-select.

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/reducers.js (right):

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:21: var newState = {};
On 2017/03/14 03:01:45, calamity wrote:
> nit: newItems

Done.

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:41: count: 0,
On 2017/03/14 03:01:45, calamity wrote:
> Is count used anywhere? Or is it just like, a #define for 0 =P

I kept meaning to either implement it or remove it for now, but forgot.

Removed for now, but we'll need it eventually.

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:55: case 'finish-search':
On 2017/03/14 03:01:45, calamity wrote:
> Also clear-search.
> 
> +Test

Done.

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:58: return
SelectionState.selectItems(selection, action);
On 2017/03/14 03:01:45, calamity wrote:
> What happens to removed anchor indexes? They'll explode on the next shift
> select, right?

Yup, again, that's something that's was fixed in the follow-up.

For now, I've made the action not assert in this case: It will just select the
single item if the anchor item is missing, and I've added a test.

https://codereview.chromium.org/2741393002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/reducers.js:270:
ClosedFolderState.updateClosedFolders = function(state, action, nodes) {
On 2017/03/14 03:01:45, calamity wrote:
> Change state to closedFolders to be consistent with the other functions.

Done.

https://codereview.chromium.org/2741393002/diff/1/chrome/test/data/webui/md_b...
File chrome/test/data/webui/md_bookmarks/reducers_test.js (right):

https://codereview.chromium.org/2741393002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/reducers_test.js:21: items: 1,
On 2017/03/14 03:01:45, calamity wrote:
> This is supposed to be an Object.

Done.

https://codereview.chromium.org/2741393002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/reducers_test.js:30: nextState =
bookmarks.SelectionState.updateSelection(initialState, action);
On 2017/03/14 03:01:45, calamity wrote:
> nextState => state as in previous patch.

Done.

[calamity, 2017-03-15 03:07:44]

lgtm.

[tsergeant, 2017-03-15 03:17:58]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2017-03-15 03:18:32]

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, 2017-03-15 04:41:10]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489547878740380,
"parent_rev": "44a3e217dedd16eea0ac435e1444e7980586f634", "commit_rev":
"f32efd59c35df91f5f63866666702c139afcac4b"}

[commit-bot: I haz the power, 2017-03-15 04:41:47]

Description was changed from

==========
MD Bookmarks: Implement item selection in new data flow system

MD Bookmarks has switched to a new data flow system which makes it
easier to bind and modify backend data from the UI. This CL implements
item selection in this new system, making it possible to select
individual items and ctrl/shift select groups of items.

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

to

==========
MD Bookmarks: Implement item selection in new data flow system

MD Bookmarks has switched to a new data flow system which makes it
easier to bind and modify backend data from the UI. This CL implements
item selection in this new system, making it possible to select
individual items and ctrl/shift select groups of items.

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

Review-Url: https://codereview.chromium.org/2741393002
Cr-Commit-Position: refs/heads/master@{#456996}
Committed:
https://chromium.googlesource.com/chromium/src/+/f32efd59c35df91f5f6386666670...
==========

[commit-bot: I haz the power, 2017-03-15 04:41:48]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/f32efd59c35df91f5f6386666670...