Inserter: Fix default block slash inserter #2771
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Regression introduced in #2323
Related: #2630
This pull request seeks to resolve an issue where the slash inserter introduced in #2630 stopped working after #2323. The issue is that because focus is now moved into a Popover when it transitions from a closed to open state, and because the Autocomplete field closes the popover when it loses focus, the popover is shown but immediately dismissed.
The changes here resolve this issue by adding a new
focusOnOpen
(default:true
) prop to thePopover
component to suppress the default focus behavior in cases such as Autocomplete where the focus behavior is not desirable, since it manages keyboard interactivity of the popover controls via the Editable input.(Note: The following changes were extracted as a separate pull request to simplify review of the minimal set of changes necessary to resolve the regressed behavior. See #2789)
To better represent the relation between the Editable and Popover search listing, props applied to the Editable have been improved using reference from thecombobox
role specification.However, this highlighted an issue: Applying arbitrary props to the Editablecontenteditable
node is not currently possible. In a few cases, we have hard-coded support for a few whitelisted props (style, className, label). Instead, the changes here refactor Editable and TinyMCE to support arbitrary prop assignment, using a "mirror" node which is leverages React reconciliation in the parent Editable component. Since we disable reconciliation for the TinyMCE component, this mirror node serves to copy attributes when changed using MutationObserver. This is made more challenging by the fact that Editable accepts many props, most of which are handled by the root Editable component and should not be assigned to the contenteditable node. While I otherwise discouragepropTypes
assignments, the usage here helps simplify picking props to assign to the mirroring node.Testing instructions:
Repeat testing instructions from #2630
Verify that there are no regressions in:Attribute assignment to TinyMCE (e.g. try applying background color to Paragraph block, which assigns itself as a combination of class and style attribute)Placeholder behavior for EditableVerify that correct ARIA expanded / ownership / active descendant attributes are assigned to both the mirror node (aria-hidden
) and the contenteditable while using the default block slash inserter.