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

Added ability to filter latest posts by author #16169

Merged

Conversation

pstonier
Copy link
Contributor

Description

Added dropdown to select author to filter the latest posts in the Latest Posts block.

How has this been tested?

Locally.

Screenshots

Screen Shot 2019-06-13 at 4 50 27 PM
Screen Shot 2019-06-13 at 4 50 39 PM

Types of changes

  • Added AuthorSelect Component to QueryControls based on the CategorySelect Component
  • Added users attribute to LatestPosts
  • LatestPosts now fetches a user list to populate the dropdown like categories

Checklist:

  • [ X ] My code is tested.
  • [ X ] My code follows the WordPress code style.
  • [ X ] My code follows the accessibility standards.
  • [ X ] My code has proper inline documentation.
  • [ n/a ] I've included developer documentation if appropriate.

@gziolo gziolo added [Block] Latest Posts Affects the Latest Posts Block Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement. [Feature] UI Components Impacts or related to the UI component system labels Jun 20, 2019
@tellthemachines tellthemachines added Needs Design Feedback Needs general design feedback. and removed Needs Technical Feedback Needs testing from a developer perspective. labels Jul 11, 2019
@talldan
Copy link
Contributor

talldan commented Jul 11, 2019

Thanks for working on this @pstonier!

The PR was discussed in a core-editor triage (https://wordpress.slack.com/archives/C02QB2JS7/p1562825386362400 - link requires slack sign up)

We decided to switch the label to Needs Design Feedback. Firstly we felt technical feedback can be provided through the normal code review process. Secondly, it'd be good to understand the motivation behind this PR (as there's no linked issue) and to make sure the UI is implemented in the right way in terms of design and behaviour of the dropdown.

@pstonier
Copy link
Contributor Author

All sounds good. From the motivation factor: we had been working on a page that was a bio about some of our consultants and needed a way to list the articles they'd authored. I had expected it to be an available filter option and when I found that it wasn't, I found my way around to adding it.

Example Page

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.

This PR effectively copies the same logic as the category filter and from my tests it appears that it works correctly. I do not approve only because of the "Needs design feedback" label, but my opinion is that we go ahead with it.

@paaljoachim
Copy link
Contributor

This feature seems straight forward to me.

Can some design folks add a comment please? As it would be nice to get it code reviewed and merged. @karmatosed @mapk @kjellr @jasmussen @melchoyce

@jasmussen
Copy link
Contributor

About to do some travelling so can't review in depth. But based on the screenshots alone, this seems good. It's an advanced feature, so it's in the sidebar, and it's a dropdown like the others. 👍 👍

@jasmussen jasmussen removed the Needs Design Feedback Needs general design feedback. label Sep 9, 2019
@paaljoachim
Copy link
Contributor

This PR looks ready for a final code review and merge. Thanks!
@talldan

@draganescu
Copy link
Contributor

In the mean time I've added #20415 implementing the same thing, with the hope of getting a faster course :) @pstonier would you look and either move this or that one towards a merge?

Copy link
Contributor

@mapk mapk left a comment

Choose a reason for hiding this comment

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

I just tested this today and it's working as expected. I like that I'm able to filter the posts by author now! Thanks. I'm approving from a design perspective.

One thing to note is now that I can filter by Author, I'd like a toggle to show the author which should probably exist here:

Screen Shot 2020-05-05 at 8 05 15 AM

Of course, don't let that hold up this PR.

@pstonier
Copy link
Contributor Author

pstonier commented May 5, 2020

I just tested this today and it's working as expected. I like that I'm able to filter the posts by author now! Thanks. I'm approving from a design perspective.

One thing to note is now that I can filter by Author, I'd like a toggle to show the author which should probably exist here:

Screen Shot 2020-05-05 at 8 05 15 AM

Of course, don't let that hold up this PR.

I like that idea and have updated the code to add that.

@pstonier pstonier requested review from nerrad and ntwb as code owners May 6, 2020 04:40
@draganescu
Copy link
Contributor

draganescu commented May 6, 2020

@pstonier @mapk #20595 adds the "Show post author" toggle in the inspector. I have not re-reviewed this yet (except one comment) to account for the new thing but will do.

@mapk
Copy link
Contributor

mapk commented May 6, 2020

When the code is dialed in again and you'd like another review, just ping me. @draganescu thanks for stepping in to help.

@pstonier pstonier requested review from draganescu and removed request for notnownikki May 28, 2020 23:09
@draganescu
Copy link
Contributor

Howdy @pstonier ,

Trying to test this, setting show author option I get:

TypeError: post.author_info is undefined edit.js:453:15

and the block crashes.

Also a rebase would be in order.

@pstonier
Copy link
Contributor Author

pstonier commented Jun 2, 2020

Howdy @pstonier ,

Trying to test this, setting show author option I get:

TypeError: post.author_info is undefined edit.js:453:15

and the block crashes.

Also a rebase would be in order.

Thanks. I have fixed this and rebased.

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.

I have tested this and works pretty well. Since we're only adding new data to the block we shouldn't need a deprecation function, testing a previously created block with this PR updated with no issues.

Great job @pstonier !

@draganescu draganescu merged commit 4ad2ed4 into WordPress:master Jun 2, 2020
@github-actions github-actions bot added this to the Gutenberg 8.3 milestone Jun 2, 2020
@pstonier
Copy link
Contributor Author

pstonier commented Jun 2, 2020

Thank you for all the help!

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 [Feature] UI Components Impacts or related to the UI component system [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants