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

Editor: Display ten most used terms #30598

Merged
merged 6 commits into from May 5, 2021

Conversation

@Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Apr 8, 2021

Description

The new component displays the 10 most used terms under the flat term selector token field. This component is only displayed when a site has ten or more terms with at least one item (post/page/CPT).

I've included accessibility fixes recommended by @afercia, except the usage count of each specific term. I omitted this aria-label since terms in the list don't have different sizes or display term count. Ref - https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/category-template.php#L941.

Fixes #8867.
Closes #21859.

Props to @olivervw and @grappler for the initial code.

Screenshots

CleanShot 2021-05-05 at 14 40 27

Types of changes

New feature

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).
@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 10, 2021

Testing using WordPress 5.7. Twenty Twenty One. Downloaded a special Gutenberg plugin containing this PR build.

This is really nice, George! Thank you!

Before:
Screen Shot 2021-04-10 at 02 07 50

After:
Screen Shot 2021-04-10 at 02 10 13

Frontend:
Screen Shot 2021-04-10 at 02 12 55

This PR also tackles this issue:
Saving tags before post creates unused/unwanted tags
#15406

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 10, 2021

I will add some issues that would be good to go through before this PR is merged.

Bug where tags disappear when adding tags in the create post view
#29077

#21589 (comment)
Associated issue: #22048

Tags and FormTokenField disappear when user don't add comma or click Enter Key
#16788
Associated PR: #16849

Suggested tags should be a list
#15291

#13282

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 23, 2021

To create a clearer distinguish between tags. You can add size differences based on how much each tag is used.
Here is a conversation about that: #8867 (comment)

Tags-show-size-difference

It is similar to how it looks using the Classic Editor.

Btw
I added the above comment with issues. To see if there is anything there that could support this PR. If it is natural to add features based on those issues to this PR. Then go ahead and add it. If not then focus on the primary tasks, and the above issues could always be followed up in another PR (if need be). What is important here is the general improvement to tags.

Loading

@Mamaduka
Copy link
Member Author

@Mamaduka Mamaduka commented Apr 27, 2021

Updated design to match Classic Editor.

most-used-terms-cloud

Same design but without border:

most-used-terms-no-border

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 27, 2021

I am wondering if we should stick to the Classic Editor framing the tags.
As it shows a very clear distinction of what is a tag and what is outside the tags area.

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 27, 2021

It looks like this PR is ready for a code review.
Thank you George for working on the tags feature! It will be very helpful to have the feature included in the Block Editor!

Loading

@vdwijngaert
Copy link
Member

@vdwijngaert vdwijngaert commented Apr 27, 2021

Going through this PR and to my untrained eye, this looks pretty nice. I do have a UX question, though: Should the "Choose from the most used Tags" button be visible when there are no terms available? Now it is, and I can imagine it can be confusing, because when no terms are available, nothing happens after clicking on the button.

We could add a fallback in the form of "You don't have any terms yet, click here to add your first", but that might be a bit redundant, because of the big fat "Add new term" form :) I'd go for hiding the button until there is at least one term to pick.

Loading

Copy link
Member

@vdwijngaert vdwijngaert left a comment

Looks good to me!

Loading

@Mamaduka Mamaduka requested a review from youknowriad Apr 28, 2021
);
};

function FlatTermSelectorMostUsed( { onSelect, taxonomy } ) {
Copy link
Contributor

@youknowriad youknowriad Apr 28, 2021

Choose a reason for hiding this comment

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

Minior: should this be extracted to its own file components/post-taxonomies/most-used-terms.js or something just to keep the size of this file contained.

Loading

Copy link
Member Author

@Mamaduka Mamaduka Apr 28, 2021

Choose a reason for hiding this comment

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

The reason didn't go with a separate file was to reuse util functions, but it might be a good idea to move them into the utils.js file.

Loading

Copy link
Member Author

@Mamaduka Mamaduka May 5, 2021

Choose a reason for hiding this comment

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

Fixed via abfa5ed.

Loading

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Apr 28, 2021

Thank you for the PR! This will be a good one to close, wow.

Here's before:

before

Here's after:

Screenshot 2021-04-28 at 11 52 39

The specific interaction:

interaction

The feature is very useful, however the tag cloud as a concept has not scaled well:

  • It makes oft-used tags easier to click and rarely used tags harder to click, as opposed to just rank them
  • It adds visual noise with the size discrepancy that isn't great for accessibility with regards to people on the autistic spectrum or dyslexics.
  • By scaling down less used tags, it can make them such low font size as to be hard to read.

The best aspect of the tag cloud is just a ranked list, which seems the simplest possible thing to do. Even something entirely basic like this would be better:

Screenshot 2021-04-28 at 12 01 42

If you rebase this PR, the is-link colors should correctly pick up your admin theme color.

20 also feels like a LOT of suggestions. Can we do with less? 6? 8? 12?

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 28, 2021

@jasmussen Joen here is a link to a discussion on why it is nice to show the tags visually in different sizes:
#8867 (comment)

Some of the discussion has to do with having similar named tags. Having a visual difference makes it easier for users to select the correct tags.

Brainstorming...
It would be helpful to have a minimum font size for the tags, and then perhaps have some "break points".
Baseline for tags at 14 px (?).
Tags used between 3-5 times the font size changes from 14px to 16px (?).
Tag is used between 6-8 times the font size changes from 16px to 18px (?).
Tags is used between 9 - 11 times the font size changes from 18px to 20px (?).

I am at present not able to download the special version of the Gutenberg plugin. Option does not show up in the left sidebar in the Checks screen.

Loading

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Apr 28, 2021

I respect that feedback, but my thoughts stand regardless. I don't see how the perceived benefit of relative sizing outweighs the accessibility downsides.

Loading

@jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Apr 28, 2021

The "choose tags..." text, and the tags themselves both appearing as links is confusing, imo. @jasmussen's suggestion looks much better to me:

Instead of using font-size to rank the tags, could we just display them in descending order of use? If that's not adequate, each tag could have a suffix that declares it's number of uses:

Most used
First tag (12), Another tag (9), Third tag (5), Last tag (1)

Loading

@getsource
Copy link
Member

@getsource getsource commented Apr 28, 2021

Personally, I have a very hard time telling when terms start and end in the flat format example (probably closest to the "don't create a wall of text" note on the link @jasmussen posted).

I like that displaying count (like @jameskoster suggests) would make it clear that they're being ordered by most used, along with providing a delimiter between terms.

I wonder if on large sites if that might lead to it looking a bit off due to the length of the number, but I suppose it'd have to be 1000+ posts before it is 4 digits.

Loading

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Apr 28, 2021

I should've also clarified that if we were to show just a plain list of tags, they should be sorted in order of usage just like Jay suggested, though I like the addition of the number count.

I wonder if on large sites if that might lead to it looking a bit off due to the length of the number, but I suppose it'd have to be 1000+ posts before it is 4 digits.

The tag plus number should probably be its own inline block so you don't accidentally get the number wrapping onto a new line without the preceeding tagname.

Loading

@jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Apr 28, 2021

it'd have to be 1000+ posts before it is 4 digits.

At this point we perhaps we can do something like "2.3k".

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 28, 2021

Please check this comment in the issue:
#8867 (comment)

(It shows various mockups.)

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented May 1, 2021

Another suggestion could be to go with what George originally suggested and add a border/frame around the tags.
Something like this:
Tags-with-frame

Then when a user hovers over a tag one will see a tooltip with the name and number of times it is used.

Tag: Example
Tooltip: Example (9)

A user will also notice that the tags are listed by what is most popular. The reason for not adding the (number) after each tag openly is that it creates visual noise. Word (number), word (number).

Tags-frame-with-numbers

A lot of the discussion here is already done in the issue. With a lot of feedback from different people. So it is gone a few rounds already. I do hope to land the Tags feature this time.

Loading

@Mamaduka Mamaduka force-pushed the try/most-used-terms-selector branch from bf46940 to abfa5ed May 5, 2021
@Mamaduka
Copy link
Member Author

@Mamaduka Mamaduka commented May 5, 2021

Pushed another design updated as suggest by @jasmussen #30598 (comment).

The tag cloud is now ranked list that displays the 10 most used terms. The label - "Most Used" comes from the taxonomy object, and this is why it doesn't match the design 100%.

CleanShot 2021-05-05 at 13 20 34

Misc changes:

  • Component now lives in a separated file.
  • I moved few utility methods into the utilts/terms.js file for convenience.

Loading

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented May 5, 2021

That looks much better, thank you @Mamaduka! This seems definitely shippable.

Question: does the interface show up always, even if you have zero or less than 10 tags total? If yes, should we limit it to showing up only when you have more than n tags, say 10?

Loading

@Mamaduka
Copy link
Member Author

@Mamaduka Mamaduka commented May 5, 2021

Thanks, @jasmussen.

Question: does the interface show up always, even if you have zero or less than 10 tags total? If yes, should we limit it to showing up only when you have more than n tags, say 10?

I think it makes sense only to display this component if a site has ten or more tags. I will update the logic.

Loading

@Mamaduka
Copy link
Member Author

@Mamaduka Mamaduka commented May 5, 2021

I've updated the display logic.

Loading

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented May 5, 2021

I am seeing this:

Screen Shot 2021-05-05 at 13 13 23

I miss having the "Choose from most used tags" text link which earlier opened up to show various tags.
Now it opens up directly to show the tags.
I see it this way:
Step 1.
User clicks to add a new tag or begins to type inside the text field box. (Does not care to see the most used tags.)
Step 2.
User wants to see the most used tags and clicks the text link to open up to see 10-15-20 etc tags.

The Tags feature has gone so many rounds. Let's just land and merge what we got. Things can be iterated upon if/when needed. Thanks for working on this feature George!

Loading

@Mamaduka Mamaduka changed the title Add most used term selector Editor: Display ten most used terms May 5, 2021
@Mamaduka
Copy link
Member Author

@Mamaduka Mamaduka commented May 5, 2021

Thanks, everyone, for testing, reviews, and suggestions ❤️

I'm going to merge the current iteration later today.

Loading

@Mamaduka Mamaduka merged commit 16918a4 into WordPress:trunk May 5, 2021
19 checks passed
Loading
@github-actions github-actions bot added this to the Gutenberg 10.6 milestone May 5, 2021
@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented May 14, 2021

I am just not getting it. Please enlighten me..:)

I installed and activated Gutenberg plugin 10.6.0 on a local test site.
Went to a All Posts screen. Clicked quick edit on a bunch of posts and added in 3 tags. The same 3 tags on 7 posts. I expected that going to any post and opening the Tags panel that I would see the Most used tags listing the 3 tags I used the most, but that did not happen.

Most-used-tags.mp4

Btw
I also logged into Make WordPress. (I am not sure if Make WordPress sites use the newest Gutenberg plugin.)

Screen Shot 2021-05-14 at 13 20 15

But I expected to see the top used tags. Independent of how many there are.

EDIT:
It is a lot easier to remember to add the correct tags when one sees these in a Most used list.
A close example from creating a Core Editor Agenda and Core Editor summary. Having the most used tags seen makes it a lot easier to remember to add the correct tags.

Loading

@Mamaduka Mamaduka deleted the try/most-used-terms-selector branch May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment