2010132 - Prevent DOM reordering in moz-box-group

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, task, P3)

Description

[Tim Giles [:tgiles]]

As part of 1972082, we need to prevent moz-box-group from reordering the DOM when the handleReorder function is called. This is because Lit will incorrectly destroy DOM nodes if we try to reorder the DOM in this function. This results in weird UI interactions, such as deleting one item in a reorderable box group will cause another item to be deleted...or sometimes no item is deleted.

:standard8 requested this work be split out from 1972082 so that the search team could track this for search shortcuts.

Comment 1

[Tim Giles [:tgiles]]

Attachment: Bug 2010132 - Allow setting-group to handle reorder events. r=#recomp-reviewers

Summary of changes: moz-box-group now emits a 'reorder' event
after reordering elements in the group, setting-control and
setting-group now handle the 'reorder' event when the
'onUserReorder' function is implemented in the setting-group
config object, moz-box-group does not reorder the DOM by default,
the reorder event object returns the same properties in both onDrop
and evaluateKeyDownEvent, a custom element wrapper was implemented
for testMozBoxGroupReorderable in order to assert the keyboard
navigation behavior of a reorderable moz-box-group.

This change allows onUserReorder to be declared in the settings group within a
settings config object. Developers can now handle reorder events through
their setting group, if that group utilizes a reorderable moz-box-group.
See Bug 1972071 for usage of the new onUserReorder function for the new
settings config.

A side effect of this change is preventing moz-box-group from reordering
the DOM by default. This was causing issues where Lit would incorrectly
DOM nodes, sometimes deleting multiple nodes, sometimes deleting nothing
instead. Because of this side effect, the existing test task,
testMozBoxGroupReorderable, needed to be modified to use a wrapper
element. This wrapper element allows us to assert the keyboard navigation
behavior, as the mouse dragging behavior was not affected by this side
effect.

Comment 2

[Tim Giles [:tgiles]]

Attachment: WIP: Bug 2010132 - reordering with keyboard is not moving focus correctly

Not entirely sure why the keyboard focus does not move to the
next/previous handle when using Ctrl + Shift + ArrowUp/Down. I'm
assuming it's because this is an asynchronous setting and so there's
some sort of timing issue. It could also be that I'm not updating the
box group items correctly, as the updateItems function never seems to
fire even when dragging or using keyboard shortcut to reorder.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/59f9d237281b
https://hg.mozilla.org/integration/autoland/rev/b10d9db3c3e0
Allow setting-group to handle reorder events. r=akulyk,hjones

Comment 4

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/b10d9db3c3e0