-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add speak message when a category gets added. #3915
Conversation
Re: noop instead of disabling the button. If I'm not wrong, disabling the button is also not fully effective to avoid multiple submission because yes the button gets disabled but the form can still be submitted multiple times pressing Enter on the input field. So I guess the whole |
Last commit doesn't disable the submit button when adding a term, to avoid a focus loss. Instead, it returns early form submission processing. This should work also when submitting the form by pressing Enter in the form fields. |
@@ -287,4 +288,4 @@ export default connect( | |||
return editPost( { [ restBase ]: terms } ); | |||
}, | |||
} | |||
)( withInstanceId( HierarchicalTermSelector ) ); | |||
)( withSpokenMessages( withInstanceId( HierarchicalTermSelector ) ) ); |
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.
Minor: At this point we might consider introducing the compose
utility to improve readability here:
import { compose } from '@wordpress/components';
// ...
const applyConnect = connect(
( state, onwProps ) => {
return {
terms: getEditedPostAttribute( state, onwProps.restBase ),
};
},
{
onUpdateTerms( terms, restBase ) {
return editPost( { [ restBase ]: terms } );
},
}
);
export default compose(
applyConnect,
withSpokenMessages,
withInstanceId
)( HierarchicalTermSelector );
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.
The only reference I see to compose is from '@wordpress/element'
not '@wordpress/components'
and it's basically flowRight
, right? I see similar cases use directly flowRight
or flow
so which should I use?
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.
The only reference I see to compose is from '@wordpress/element' not '@wordpress/components'
Correct, I was mistaken.
and it's basically flowRight, right?
Yes. I think the original reasoning was that if we're emphasizing this pattern of higher-order components, it should be made a first-class part of the element abstraction.
I see similar cases use directly flowRight or flow so which should I use?
Since #3907 all instances should have been updated to use compose
.
If you're curious, there's a bit of detail at #2500 (comment) which explains the difference between flow
and flowRight
in the context of higher-order components.
@@ -114,7 +114,9 @@ class HierarchicalTermSelector extends Component { | |||
.then( ( term ) => { | |||
const hasTerm = !! find( this.state.availableTerms, ( availableTerm ) => availableTerm.id === term.id ); | |||
const newAvailableTerms = hasTerm ? this.state.availableTerms : [ term, ...this.state.availableTerms ]; | |||
const { onUpdateTerms, restBase, terms } = this.props; | |||
const { onUpdateTerms, restBase, terms, slug } = this.props; | |||
const termAddedMessage = slug === 'category' ? __( 'Category added' ) : __( 'Term added' ); |
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.
Too bad there's not a taxonomy label for this so we didn't need to hard-code it 😕
https://developer.wordpress.org/reference/functions/get_taxonomy_labels/
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.
Maybe we can open a Trac ticket for this 🙂
This PR tries to improve the accessibility of the "Add Category" form. It adds a
speak
message when a category gets added.Still to address: try to noop the button instead of disabling it.
Fixes #2634