Skip to content
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

Adds multi-select to categories on Latest Posts #20781

Merged

Conversation

Ringish
Copy link
Contributor

@Ringish Ringish commented Mar 11, 2020

Description

Adds possibility to add & select posts in LatestPosts by multiple categories as described in #20046.

How has this been tested?

Added a couple of posts and categories, and used the new implementation to test, edit and save.

Screenshots

ezgif-3-7d71cf866e74

Types of changes

Adds ability to filter by multiple categories. May be breaking change? Probably this could break blocks with the old implementation.

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
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 work @Ringish :) Left some comments about the code. As a general note i advise you to follow the setup in the code contribution guide to benefit from automatic linting and early problem detection.

Setting up locally Docker and the @wordpress/wp-env package will also allow you to run failing tests locally and debug.

Looking forward for testing this again after you've went through my suggestions! 🚀

packages/block-library/src/latest-posts/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/edit.js Outdated Show resolved Hide resolved
@Ringish
Copy link
Contributor Author

Ringish commented Mar 11, 2020

Great work @Ringish :) Left some comments about the code. As a general note i advise you to follow the setup in the code contribution guide to benefit from automatic linting and early problem detection.

Setting up locally Docker and the @wordpress/wp-env package will also allow you to run failing tests locally and debug.

Looking forward for testing this again after you've went through my suggestions! 🚀

Really good feedback. Thank you! I've pushed a new commit.

@draganescu draganescu self-requested a review March 11, 2020 15:43
@draganescu draganescu self-requested a review March 11, 2020 15:44
@draganescu draganescu merged commit d0ca255 into WordPress:master Mar 11, 2020
@github-actions github-actions bot added this to the Gutenberg 7.8 milestone Mar 11, 2020
@draganescu
Copy link
Contributor

Tested this locally by filtering on multiple categories and LGTM!

@paaljoachim
Copy link
Contributor

paaljoachim commented Mar 12, 2020

Will we still see the list of possible categories below the multiselect box?
So we know which categories we can choose from.

@draganescu
Copy link
Contributor

@paaljoachim not at this point, no. That is a disatvantage of this tokenized system. When implementing such a system you basically trat categories like tags and type two letters to get a fuzzy match list.

On another note, I will open an issue to move the control token select to QueryControls to respect the current implementation style.

@@ -96,6 +97,22 @@ class LatestPostsEdit extends Component {
featuredImageSizeWidth,
featuredImageSizeHeight,
} = attributes;
const suggestions = categoriesList.reduce(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should rename suggestions to categorySuggestions

@mcsf
Copy link
Contributor

mcsf commented Apr 15, 2020

The change of attribute schema for this block here should have been accompanied with provisions for migration, especially considering that this is a dynamic block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Posts Affects the Latest Posts Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants