Skip to content

Moves category multi select from LatestPosts to QueryControls#20832

Merged
draganescu merged 9 commits intoWordPress:masterfrom
Ringish:update/categories-select-multiple
Mar 16, 2020
Merged

Moves category multi select from LatestPosts to QueryControls#20832
draganescu merged 9 commits intoWordPress:masterfrom
Ringish:update/categories-select-multiple

Conversation

@Ringish
Copy link
Copy Markdown
Contributor

@Ringish Ringish commented Mar 12, 2020

Description

Solves #20826.

How has this been tested?

Screenshots

Skärmavbild 2020-03-12 kl  14 39 23

Types of changes

File structure fix!

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Copy link
Copy Markdown
Contributor

@draganescu draganescu left a 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!

onOrderChange,
onOrderByChange,
} ) {
const suggestions = categoriesList.reduce(
Copy link
Copy Markdown
Contributor

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 ) }
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

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?

Copy link
Copy Markdown
Contributor

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;

@Ringish
Copy link
Copy Markdown
Contributor Author

Ringish commented Mar 13, 2020

I think we should merge this before I move on with #20831.

Copy link
Copy Markdown
Contributor

@draganescu draganescu left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants