Keyboard._restoreFocus() clears the active selection in 'extended' mode

[dmartin-gh]

If the focused row/cell receives an update from the store, the existing selection will be cleared during the focus restoration process. This only affects the 'extended' selection mode.

This is easily reproducible in the dgrid/test/Selection.html page.

1. Load the page and scroll down to the second grid titled A grid with cell-level selection (default selection mode of "extended").
2. Hold SHIFT and make a selection from Row5,Col1 to Row0,Col1 (the first 5 vertical cells in the first column). You now have Row0,Col1 focused as it was the last one you clicked.
3. Open the console and run gridCellNavigation.store.put(gridCellNavigation.store.get(1)). The selection remains intact.
4. Now run gridCellNavigation.store.put(gridCellNavigation.store.get(0)). The selection is cleared and only Row0,Col1 remains selected.

When Keyboard._restoreFocus() runs, it fires the dgrid-cellfocusin event. Selection._extendedSelectionHandler() picks this up and clears the active selection.

[dmartin-gh]

I've attached a potential fix for this behavior. The _restoreFocus() method now includes the previous target in the dgrid-cellfocusin event. The _extendedSelectionHandler() method now skips the clearSelection() call if the target of the event is the same as the old target. This does not affect _restoreFocus() calls where a sibling is now focused (the target changed).

A drawback to this is the change from focus() to _focusOnNode() in _restoreFocus(). Mixins that have overridden the focus() method will no longer get their code executed during focus restoration. An alternative solution is to allow event-muckery at the focus() API level via a second optional event argument.

[dmartin-gh]

Has anyone had a chance to look at this?

[kfranqueiro]

Not yet, sorry, but I hope to get a chance to look at this and one or two other PRs next week if time permits.

[kfranqueiro]

To be honest, my first thought looking at this again is that I wonder if in-place re-focusing should even emit dgrid's focus events to begin with. As far as the user's concerned their focus didn't budge an inch, right? I imagine if we made the event not emit at all in that case, this problem would also go away?

[dmartin-gh]

That's probably fine, as long as something else triggers the re-focus of the new row element under the covers.

Edit: Never mind, it's already been refocused at the point the event is emitted. Suppressing the event should just solve it.