Templates: Add back button & fix focus loss when navigating through template creation flow #70091
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
I would like some feedback on PR and on issue with other focus loss: Here is video, that better shows the focus loss when going to post based templates Let me know if we should fix these in this PR itself |
There was a problem hiding this comment.
Thanks for the PR! I think this PR is a great a11y improvement, but we need to address two more things:
First, we need the back button in the "Add template: Post" modal as well:
Second, when the Back button is pressed, focus is lost. We should focus either the modal itself or the first focusable element within the modal.
packages/edit-site/src/components/add-new-template/add-custom-generic-template-modal-content.js
Outdated
Show resolved
Hide resolved
|
Hey @t-hamano, I have added the back button in all steps and fixed focus loss issue. https://q7utzrengv.ufs.sh/f/Wgl9eBAmTj29pojunySmx9PaNdTZpH58C7GnvcBY3RjewJus Can you check? Also should we focus heading or first tab item? I feel that currently it just loops 3 times "Add template" when creating posts/page templates. |
| // Focus on the first focusable element when the modal opens. | ||
| // We handle focus management in the parent modal, just need to focus on the first focusable element. | ||
| useEffect( () => { | ||
| if ( containerRef.current ) { | ||
| const [ firstFocusable ] = focus.focusable.find( | ||
| containerRef.current | ||
| ); | ||
| firstFocusable?.focus(); | ||
| } | ||
| }, [ showSearchEntities ] ); |
There was a problem hiding this comment.
I'm thinking that we could centralize all focus management in the modal, i.e. NewTemplateModal. I tried the following code.
Details
diff --git a/packages/edit-site/src/components/add-new-template/add-custom-generic-template-modal-content.js b/packages/edit-site/src/components/add-new-template/add-custom-generic-template-modal-content.js
index dd8ccce86f..e5b21c8633 100644
--- a/packages/edit-site/src/components/add-new-template/add-custom-generic-template-modal-content.js
+++ b/packages/edit-site/src/components/add-new-template/add-custom-generic-template-modal-content.js
@@ -6,7 +6,7 @@ import { paramCase as kebabCase } from 'change-case';
/**
* WordPress dependencies
*/
-import { useState, useEffect, useRef } from '@wordpress/element';
+import { useState, useRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import {
Button,
@@ -21,13 +21,6 @@ function AddCustomGenericTemplateModalContent( { createTemplate, onBack } ) {
const [ isBusy, setIsBusy ] = useState( false );
const inputRef = useRef();
- // Set focus to the name input when the component mounts
- useEffect( () => {
- if ( inputRef.current ) {
- inputRef.current.focus();
- }
- }, [] );
-
async function onCreateTemplate( event ) {
event.preventDefault();
if ( isBusy ) {
diff --git a/packages/edit-site/src/components/add-new-template/add-custom-template-modal-content.js b/packages/edit-site/src/components/add-new-template/add-custom-template-modal-content.js
index 7a846985c8..3b0d9b998c 100644
--- a/packages/edit-site/src/components/add-new-template/add-custom-template-modal-content.js
+++ b/packages/edit-site/src/components/add-new-template/add-custom-template-modal-content.js
@@ -16,7 +16,6 @@ import {
import { useEntityRecords } from '@wordpress/core-data';
import { decodeEntities } from '@wordpress/html-entities';
import { useDebouncedInput } from '@wordpress/compose';
-import { focus } from '@wordpress/dom';
/**
* Internal dependencies
@@ -170,23 +169,11 @@ function AddCustomTemplateModalContent( {
onSelect,
entityForSuggestions,
onBack,
- containerRef,
} ) {
const [ showSearchEntities, setShowSearchEntities ] = useState(
entityForSuggestions.hasGeneralTemplate
);
- // Focus on the first focusable element when the modal opens.
- // We handle focus management in the parent modal, just need to focus on the first focusable element.
- useEffect( () => {
- if ( containerRef.current ) {
- const [ firstFocusable ] = focus.focusable.find(
- containerRef.current
- );
- firstFocusable?.focus();
- }
- }, [ showSearchEntities ] );
-
return (
<VStack
spacing={ 4 }
diff --git a/packages/edit-site/src/components/add-new-template/index.js b/packages/edit-site/src/components/ad
d-new-template/index.js
index aa36fa2e59..5ad4f11ff0 100644
--- a/packages/edit-site/src/components/add-new-template/index.js
+++ b/packages/edit-site/src/components/add-new-template/index.js
@@ -184,10 +184,7 @@ function NewTemplateModal( { onClose } ) {
useEffect( () => {
// Focus the first focusable element when component mounts or UI changes
// We don't want to focus on the other modals because they have their own focus management.
- if (
- containerRef.current &&
- modalContent === modalContentMap.templatesList
- ) {
+ if ( containerRef.current ) {
const [ firstFocusable ] = focus.focusable.find(
containerRef.current
);
@@ -339,7 +336,6 @@ function NewTemplateModal( { onClose } ) {
onBack={ () =>
setModalContent( modalContentMap.templatesList )
}
- containerRef={ containerRef }
/>
) }
{ modalContent === modalContentMap.customGenericTemplate && (This ensures that the modal dialog itself consistently gets focus when the modal content is switched. What do you think?
There was a problem hiding this comment.
There are two reasons, I did it this way:
- We wanted to focus on input field when its present, its related to this comment.
In my opinion, the solution to this would be to set focus to the "Name" input, and also add a 'Back' option, so that a user can cancel the Create Custom Template action without having to start over. This should be true for all templates: selecting "Category Archives", for example, should provide an option to return to the selection screen.
- This works fine but we will then need to move state of
const [ showSearchEntities, setShowSearchEntities ] = useState(
entityForSuggestions.hasGeneralTemplate
);to parent component, as there is also focus loss when going from "Select whether to create a single template for all items or a specific one." to "This template will be used only for the specific item chosen." and back. This happens as useEffect does not have it as dependancy in parent block.
There was a problem hiding this comment.
I see, in the AddCustomTemplateModalContent component, the content switches depending on the showSearchEntities state, so this focus management is necessary.
…70091) * feat: add focus to name input and back navigation * refactor: simplify focus handling for name input in template modal * feat: add back navigation functionality to custom template modal * feat: enhance focus management in create template modals Co-authored-by: Mayank-Tripathi32 <mayanktripathi32@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: Sourav61 <sourav08@git.wordpress.org>
Added by @t-hamano
Note
Don't forget to add @Sourav61, who worked in #69711, as a co-author when merging this PR.
What?
Closes #69711
Adds a Back button to the template creation flow and sets focus on the name input field when creating a custom template.
Why?
When clicking the "Custom template" button, the focus is currently lost, causing accessibility issues for screen reader users. This PR fixes the focus management and improves the navigation flow by adding a Back button that allows users to return to the previous step without closing the entire modal.
How?
Testing Instructions
Testing Instructions for Keyboard
Screencast
https://q7utzrengv.ufs.sh/f/Wgl9eBAmTj29r0ltzJ9bvXLPnVf0wHAxqleM96uGDdWoCa3T