Block editor: placeholders: try admin shadow #33494
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { | ||
| useRefEffect, | ||
| useConstrainedTabbing, | ||
| useMergeRefs, | ||
| } from '@wordpress/compose'; | ||
| import { useState, createPortal } from '@wordpress/element'; | ||
| import { ENTER, SPACE, ESCAPE } from '@wordpress/keycodes'; | ||
| import { focus } from '@wordpress/dom'; | ||
| import { __experimentalStyleProvider as StyleProvider } from '@wordpress/components'; | ||
|
|
||
| /** | ||
| * Embeds the given children in shadow DOM that has the same styling as the top | ||
| * window (admin). A button is returned to allow the keyboard user to enter this | ||
| * context. Visually, it appears inline, but it is styled as the admin, not as | ||
| * the editor content. | ||
| * | ||
| * @param {Object} props Button props. | ||
| * | ||
| * @return {WPComponent} A button to enter the embedded admin context. | ||
| */ | ||
| export default function EmbeddedAdminContext( props ) { | ||
| const [ shadow, setShadow ] = useState(); | ||
| const [ hasFocus, setHasFocus ] = useState(); | ||
| const ref = useRefEffect( ( element ) => { | ||
| const root = element.attachShadow( { mode: 'open' } ); | ||
|
|
||
| // Copy all admin styles to the shadow DOM. | ||
| const style = document.createElement( 'style' ); | ||
| Array.from( document.styleSheets ).forEach( ( styleSheet ) => { | ||
| // Technically, it's fine to include this, but these are styles that | ||
| // target other components, so there's performance gain in not | ||
| // including them. Below, we use `StyleProvider` to render emotion | ||
| // styles in shadow DOM. | ||
| if ( styleSheet.ownerNode.getAttribute( 'data-emotion' ) ) { | ||
| return; | ||
| } | ||
|
|
||
| // Try to avoid requests for stylesheets of which we already | ||
| // know the CSS rules. | ||
| try { | ||
| let cssText = ''; | ||
|
|
||
| for ( const cssRule of styleSheet.cssRules ) { | ||
| cssText += cssRule.cssText; | ||
| } | ||
|
|
||
| style.textContent += cssText; | ||
| } catch ( e ) { | ||
| root.appendChild( styleSheet.ownerNode.cloneNode( true ) ); | ||
| } | ||
| } ); | ||
| root.appendChild( style ); | ||
| setShadow( root ); | ||
|
|
||
| function onFocusIn() { | ||
| setHasFocus( true ); | ||
| } | ||
|
|
||
| function onFocusOut() { | ||
| setHasFocus( false ); | ||
| } | ||
|
|
||
| /** | ||
| * When pressing ENTER or SPACE on the wrapper (button), focus the first | ||
| * tabbable inside the shadow DOM. | ||
| * | ||
| * @param {KeyboardEvent} event The keyboard event. | ||
| */ | ||
| function onKeyDown( event ) { | ||
| if ( element !== event.path[ 0 ] ) return; | ||
| if ( event.keyCode !== ENTER && event.keyCode !== SPACE ) return; | ||
|
|
||
| event.preventDefault(); | ||
|
|
||
| const [ firstTabbable ] = focus.tabbable.find( root ); | ||
| if ( firstTabbable ) firstTabbable.focus(); | ||
| } | ||
|
|
||
| /** | ||
| * When pressing ESCAPE inside the shadow DOM, focus the wrapper | ||
| * (button). | ||
| * | ||
| * @param {KeyboardEvent} event The keyboard event. | ||
| */ | ||
| function onRootKeyDown( event ) { | ||
| if ( event.keyCode !== ESCAPE ) return; | ||
|
|
||
| root.host.focus(); | ||
| event.preventDefault(); | ||
| } | ||
|
|
||
| let timeoutId; | ||
|
|
||
| /** | ||
| * When clicking inside the shadow DOM, temporarily remove the ability | ||
| * to catch focus, so focus moves to a focusable parent. | ||
| * This is done so that when the user clicks inside a placeholder, the | ||
| * block receives focus, which can handle delete, enter, etc. | ||
| */ | ||
| function onMouseDown() { | ||
| element.removeAttribute( 'tabindex' ); | ||
| timeoutId = setTimeout( () => | ||
| element.setAttribute( 'tabindex', '0' ) | ||
| ); | ||
| } | ||
|
|
||
| root.addEventListener( 'focusin', onFocusIn ); | ||
| root.addEventListener( 'focusout', onFocusOut ); | ||
| root.addEventListener( 'keydown', onRootKeyDown ); | ||
| element.addEventListener( 'keydown', onKeyDown ); | ||
| element.addEventListener( 'mousedown', onMouseDown ); | ||
|
Comment on lines
+110
to
+114
There was a problem hiding this comment. This feels very awkward to use. I had to look at this patch to figure out that I could press Enter or Space at the right moment and then Tab to reach the placeholder controls. And there is no indication that Enter or Space did anything related to focus (Space even scrolls the page), and on Safari 15.1 I sometimes need to press ↓ not once, but twice, when moving into the Table block from above. So there is no membrane visible to the eye, but there is one to the fingers. This should be made consistent: either there is always a clear membrane, either there is never one. I'd prefer the latter. There was a problem hiding this comment.
Doesn't focus move to the first element in the placeholder?
There has always been a membrane of some sort. Tab works in placeholders but doesn't work in the content (to tab within the content). What do you suggest by removing the membrane? There was a problem hiding this comment. I shared my screen with Ella in private, and I think she'll put up some notes soon. In the meantime, for other readers:
The focus is never apparent to the visual keyboard user. Only once one of the placeholder's controls is focused do I see visual feedback of that focus.
I was coming from the position that the placeholder, while not really content, doesn't communicate that it should behave differently from content when it comes to keyboard navigation and focus. And, visually, there's not enough indication that the focus is on a different type of UI when the placeholder has the focus. That's what I meant by "a visual membrane not existing". From that interpretation it followed that there should also not be a membrane or hindrance when navigating with the keyboard. |
||
| return () => { | ||
| root.removeEventListener( 'focusin', onFocusIn ); | ||
| root.removeEventListener( 'focusout', onFocusOut ); | ||
| root.removeEventListener( 'keydown', onRootKeyDown ); | ||
| element.removeEventListener( 'keydown', onKeyDown ); | ||
| element.removeEventListener( 'mousedown', onMouseDown ); | ||
| clearTimeout( timeoutId ); | ||
| }; | ||
| }, [] ); | ||
ellatrix marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const dialogRef = useRefEffect( ( element ) => { | ||
| if ( | ||
| element.getRootNode().host !== element.ownerDocument.activeElement | ||
| ) | ||
| return; | ||
|
|
||
| const [ firstTabbable ] = focus.tabbable.find( element ); | ||
| if ( firstTabbable ) firstTabbable.focus(); | ||
| }, [] ); | ||
|
|
||
| const content = ( | ||
| <StyleProvider document={ { head: shadow } }> | ||
| <div | ||
| role="dialog" | ||
| ref={ useMergeRefs( [ useConstrainedTabbing(), dialogRef ] ) } | ||
| > | ||
| { props.children } | ||
| </div> | ||
| </StyleProvider> | ||
| ); | ||
|
|
||
| return ( | ||
| <div | ||
| { ...props } | ||
| ref={ ref } | ||
| tabIndex={ 0 } | ||
| role="button" | ||
| aria-pressed={ hasFocus } | ||
| > | ||
| { shadow && createPortal( content, shadow ) } | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { Placeholder } from '@wordpress/components'; | ||
| import { __, sprintf } from '@wordpress/i18n'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import EmbeddedAdminContext from '../embedded-admin-context'; | ||
|
|
||
| /** | ||
| * Placeholder for use in blocks. Creates an admin styling context and a tabbing | ||
| * context in the block editor's writing flow. | ||
| * | ||
| * @param {Object} props | ||
| * | ||
| * @return {WPComponent} The component | ||
| */ | ||
| export default function IsolatedPlaceholder( props ) { | ||
| return ( | ||
| <EmbeddedAdminContext | ||
| aria-label={ sprintf( | ||
| /* translators: %s: what the placeholder is for */ | ||
| __( 'Placeholder: %s' ), | ||
| props.label | ||
| ) } | ||
| className="wp-block-editor-placeholder" | ||
| > | ||
| <Placeholder { ...props } /> | ||
| </EmbeddedAdminContext> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels weird to target
data-emotion. That's sounds like an implementation detail that shouldn't be relied upon.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, ideally we should only include the styles that are needed for the placeholder, but that's very hard to do. Not sure what else we can do here, aside from just including everything. It's not really a problem to include the styles, it's just bad for performance.