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

Dashboard: Add Filter by Taxonomy #11436

Closed
sblinde opened this issue May 4, 2022 · 15 comments · Fixed by #11625
Closed

Dashboard: Add Filter by Taxonomy #11436

sblinde opened this issue May 4, 2022 · 15 comments · Fixed by #11625
Assignees
Labels
Group: Dashboard Package: WP Dashboard /packages/wp-dashboard Type: Enhancement New feature or improvement of an existing feature

Comments

@sblinde
Copy link
Contributor

sblinde commented May 4, 2022

Summary

This issue will add a new filter option to the Dashboard to filter Stories, based on hierarchical taxonomy.

Screen Shot 2022-04-20 at 11 42 53 AM

References

UX: Still in progress, but this issue's implementation can be started prior to having fleshed out comps for this. We can mimic the experience described below.

For an example of what this could look like, please see All Stories in the default WordPress view for what this means. The functionality between these two spaces should be the same: a dropdown, similar to the Author in the screenshot seen above.

Alternatives Considered

No alternatives.

Acceptance Criteria

Does this epic have any performance impact?

This should not impact performance.

Does this epic have telemetry?

We could capture usage of this dropdown via telemetry.

@sblinde sblinde added Type: Enhancement New feature or improvement of an existing feature Group: Dashboard Pod: Pea Package: WP Dashboard /packages/wp-dashboard labels May 4, 2022
@spacedmonkey
Copy link
Contributor

For reference, the post list page in WordPress looks like this.
Screenshot 2022-05-05 at 17 58 11

@sblinde sblinde self-assigned this May 9, 2022
@sblinde
Copy link
Contributor Author

sblinde commented May 12, 2022

Per conversation earlier this week with Amy, we'll lean towards renaming the dropdown to Categories similar to the categories dropdown. Taxonomies usually includes both Categories and Tags, but the use case here is for Categories, and because there are custom taxonomies that can be a part of WordPress, best if we're clear on what the filter is meant for to reduce confusion!

@mwritter mwritter assigned mwritter and unassigned sblinde May 18, 2022
@sblinde sblinde self-assigned this May 20, 2022
@felipebochehin87
Copy link

@mwritter @sblinde @swissspidy

The functionality is ok, it's filtering stories based on the value selected in the filter. But please, one thing that is not clear, which should be the name of the dropdown: taxonomies or categories? I currently see "Taxonomies", but there's a comment from 15 days ago saying that the name should be "Categories". What's the expected? Thanks!

dropdown.png

@spacedmonkey
Copy link
Contributor

Why was this PR moved to QA? It does seem to have been approved by anyone.

@mwritter
Copy link
Contributor

Categories

#11526 (comment)
@felipebochehin87 per the conversation on the PR, after our design meeting we were under the impression that this filter was only going to be used for filtering 'Categories'. It is now understood that this filter is for 'hierarchical' taxonomies, so the name 'Category' would not really work here anymore. 🤔It should probably just be 'Taxonomy' though not 'Taxonomies'.

@mwritter
Copy link
Contributor

Why was this PR moved to QA? It does seem to have been approved by anyone.

#11526 (comment)
@spacedmonkey per the conversation on the PR, @sblinde mentioned that we were going to move to qa while we work on writing a couple more tests for test coverage. 🤔 There should have been a review on it though I do agree, @sblinde I know we discussed the code change could you give it a formal review when you get a chance? - the main code change shouldn't change only test coverage is needed.

@spacedmonkey
Copy link
Contributor

@mwritter This is not the normal process in this project. PRs must be approved before they go to QA. Also, someone from the WP pod should really review and approve this PR, as this PR touches WordPress apis.

@spacedmonkey
Copy link
Contributor

Having a quick look at the functionality of this PR, it is a little confusing. Why is this dropdown a mixture of taxonomy terms from different taxonomies. If you had a term of rock, in two taxonomies, how would you know which one you are selecting here? I believe it best to have a dropdown per taxonomy, not one dropdown for everything. This mirrors how WP list page does it.
Screenshot 2022-05-27 at 14 25 51

@sblinde
Copy link
Contributor Author

sblinde commented May 27, 2022

@spacedmonkey Understood, I'm used to a more flexible QA process where QA can be either prior to or after approval, so I apologize. I had given an approval prior to the further changes to the PR being done, and also had helped throughout in parts to the PR myself as well. I tried to explain in the original PR: being newer to this project, wanted to make sure we had this working well given the confusion around custom taxonomies and what the original ask was for.

I do agree: someone from WP pod should review it (and by no means were we asking to have QA done and have this ready to merge, we all understand adequate approvals are required prior to that).

@sblinde
Copy link
Contributor Author

sblinde commented May 27, 2022

Having a quick look at the functionality of this PR, it is a little confusing. Why is this dropdown a mixture of taxonomy terms from different taxonomies. If you had a term of rock, in two taxonomies, how would you know which one you are selecting here? I believe it best to have a dropdown per taxonomy, not one dropdown for everything. This mirrors how WP list page does it. Screenshot 2022-05-27 at 14 25 51

@spacedmonkey Totally understand what you mean and excellent point, and I think that's something that wasn't clear for us picking up on the prior work slated by the previous team. At first, we thought that we were supposed to be supporting Categories (as seen in that dropdown), and then the ask was that we include All Taxonomies. Your point is right: if you had more than one same-named custom taxonomy, it would show up similarly in the dropdown.

But as far as I'm aware, even with adding custom taxonomies within Stories and that similar page view, that there's not a space that actually shows any dropdowns for custom taxonomies, not even within the WP list style page for stories as seen below in my screenshot.

Screen Shot 2022-05-27 at 10 03 41 AM
(Note: this one Story has two custom taxonomies assigned to it and one category, but there's no indication or view to know such within this page, and no category dropdown)

But I would have hoped this would have come up with initial conversations about designing and planning this work prior to our start implementing it. Our team is pretty fresh at this work, so it's harder for us to know (especially since our initial thought was that we were supporting categories) what's expected if it isn't clearly defined and scoped. If there are gaps between what's expected, those gaps will be larger for us as a team with fresh eyes coming into this.

@sblinde
Copy link
Contributor Author

sblinde commented May 30, 2022

For clarification based on conversations, here are the expectations for this feature:

  • This feature should support multiple taxonomies, including custom taxonomies that are defined as hierarchical (seen as: hierarchical: true coming from the Taxonomies endpoint)
  • This feature should render an individual Taxonomy dropdown for each type of hierarchical taxonomy, and each filter should work independently of one another.
    • Example: Imagine a WordPress instance that had Categories, Tags, and Colors (the latter being a custom taxonomy). Tags would not have a dropdown, because Tags has hierarchical: false. Categories and Colors would have individual dropdowns on the Dashboard, respective of each item.
    • The title of each dropdown should correspond to the name of the type (i.e. Categories would be titled All Categories)

@swissspidy
Copy link
Collaborator

The title of each dropdown should correspond to the name of the type

For context, the right All Categories labels will be provided in the REST API response under labels.allitems. See how the story-editor package uses taxonomy.labels.[...] everywhere as well.

There's no need to provide this label manually (and string concatenation like "All %s" should be avoided anyway)

@spacedmonkey
Copy link
Contributor

Should we respect the "show ui" or "show admin column" visibility fields?
Screenshot 2022-06-09 at 12 05 35

@swissspidy
Copy link
Collaborator

Should we respect the "show ui" or "show admin column" visibility fields?

Yeah good call, I think we can filter by visibility.show_ui (not the other one).

We do that in the editor as well:

const availableTaxonomies = taxonomies.filter((taxonomy) => {
const isVisible = taxonomy?.visibility?.show_ui;
const canAssignTerms = Boolean(
capabilities[`assign-${taxonomy?.restBase}`] ||
capabilities[`assign-${taxonomy?.slug}`]
);
return isVisible && canAssignTerms;
});

^ @mwritter It should be easy to implement that in the same spot where you currently filter hierarchical taxonomies.

(ignore the canAssignTerms part!)

Let's continue the discussion about that on the PR though.

@sblinde sblinde removed their assignment Jun 13, 2022
@felipebochehin87
Copy link

Tested filters and combinations. Working fine.

Verticals
Categories
Authors
Verticals + Categories
Verticals + Authors
Categories + Authors
Verticals + Categories + Authors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Dashboard Package: WP Dashboard /packages/wp-dashboard Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
5 participants