-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Moves category multi select from LatestPosts to QueryControls #20832
Moves category multi select from LatestPosts to QueryControls #20832
Conversation
…date/categories-select-multiple
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.
Awesome and super fast @Ringish :) Left two minor comments, thanks!
@@ -37,6 +37,14 @@ export default function QueryControls( { | |||
onOrderChange, | |||
onOrderByChange, | |||
} ) { | |||
const suggestions = categoriesList.reduce( |
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.
Let's call this categorySuggestions
because if we add other flat taxonomies they'll also have suggestions.
value: item.name || item.value, | ||
} ) ) | ||
} | ||
suggestions={ Object.keys( suggestions ) } |
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 FormTokenField
has a maxSuggestions
prop what we should use for cases when categories are not for content and so are too many in number.
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.
What value should be max?
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.
Let's use a constant and the same value the FlatTermSelector
component is using:
const MAX_TERMS_SUGGESTIONS = 20;
I think we should merge this before I move on with #20831. |
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.
Great @Ringish I will merge this to clean up the component and lessen the future conflicts.
Description
Solves #20826.
How has this been tested?
Screenshots
Types of changes
File structure fix!
Checklist: