-
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
Editor: Fix term selector component exports #41784
Conversation
Size Change: +16 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
This effectively removes the ability for third parties to filter |
@mcsf, one other option I can think of is to update the README example, but that won't prevent an infinite loop when used with the |
Thanks, that's useful context. I spent some time digging up the trail of issues leading up to where we are.
// DO NOT USE THIS
function customizeTaxonomySelector( OriginalComponent ) {
return function( props ) {
if ( 'my_taxonomy' !== props.slug ) {
return <OriginalComponent { ...props } />;
}
return <HierarchicalTermSelector { ...props } />;
};
}
wp.hooks.addFilter( 'editor.PostTaxonomyType', 'my-custom-plugin', customizeTaxonomySelector ); The snippet above is instructing the editor to always use the hierarchical selector when handling gutenberg/packages/editor/src/components/post-taxonomies/index.js Lines 36 to 38 in 739f26f
Custom taxonomies should make sure they register themselves as hierarchical.
Honestly, I think that's the most correct and safest thing to do. I mean, the example needs to be changed anyway, per my previous point about Then, if someone really wants to be able to hook into export FlatTermSelector as UnfilteredFlatTermSelector;
export default withFilters( 'editor.PostTaxonomyType' )( FlatTermSelector ); or const FilteredFlatTermSelector = withFilters( 'editor.PostTaxonomyType' )( FlatTermSelector );
FilteredFlatTermSelector.Unfiltered = FlatTermSelector;
export default FilteredFlatTermSelector; To reiterate, I think this PR should pivot to fixing the documentation and accompanying example. On the other hand, the idea of exporting undecorated components is something we should be careful about, and only consider in a new PR/issue if there is pressure to do so. wdyt, @Mamaduka? |
@mcsf, the documentation update makes sense to me. So I will try to come up with something simple and update the PR. |
As mentioned in #17476, some custom taxonomies need to be non-hierarchical but should still be selected by checkboxes. For example for pre-defined list of items where an author can't add more items. Looking at a project, which is still using a copy of I think the proposed change in this PR makes sense since it appears that the current exports are causing breakage and back-compat shouldn't be a real issue here. |
@mcsf, I would prefer to go with my initial fix. |
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.
Looks good to me, fix seems to be an oversight from #38419. Not sure if we need some tests here so this doesn't happen again?
I'm not sure about the tests. Let's merge this fix, and happy to a follow-up if I can think of a good way to test the exports. |
What?
Fixes #41661.
PR directly exports
FlatTermSelector
andHierarchicalTermSelector
components without applying the filter.Why?
The developers have to use the same filter to change components used for rendering the taxonomy, causing an infinite loop.
Testing Instructions
HierarchicalTermSelector
component.