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 keywords to Spoiler block to improve its discoverability #59636

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

juanmaguitar
Copy link

@juanmaguitar juanmaguitar commented Dec 30, 2021

Changes proposed in this Pull Request

Added new keywords to the "spoiler" keyword to improve its discoverability

p4hfux-5YS-p2#comment-8798

__( 'spoiler' ),
__( 'accordion' ),
__( 'Dropdown' ),
__( 'details' ),
Copy link

Choose a reason for hiding this comment

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

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 38 times:
translate( 'Details:' ) ES Score: 8
See 1 additional suggestions in the PR translation status page

@juanmaguitar
Copy link
Author

juanmaguitar commented Jan 12, 2022

To add more context on this Issue, it seems there was a limit of 3 keywords when registering a block that was removed at WordPress/gutenberg#13848

PS: From the PR desc it seems the PR aimed to use for searches only the first 3 keywords registered, but judging from the comments below, it seemed there was no cut-off anymore at all

The @wordpress/blocks package is the one in charge of this and the change mentioned was merged on Sep 2019. Several versions of the package has been released from that date and wp-calypso seem to use the 11.1.4 version that should include this removal of the 3 keywords limit

Thanks @ockham for the feedback on this issue

@simison simison added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed OSS Citizen labels Jan 14, 2022
@simison
Copy link
Member

simison commented Jan 14, 2022

Yep, limit of 3 is no more. 🎉

The "spoiler" keyword is already in title so it's probably not doing anything in the keyword list, not a biggie tho.

@lezama Since this is a P2 block, could someone from Lighthouse help test & merge to P2? Should be really quickie. Thank you! 🙌

(Aside, maybe these blocks could be moved to P2 repo?)

Copy link
Member

@simison simison left a comment

Choose a reason for hiding this comment

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

Have not test-run but code looks good.

Thank you!

@simison simison requested a review from a team January 14, 2022 09:23
@lamosty
Copy link
Contributor

lamosty commented Jan 14, 2022

@simison thanks for the ping!

I might be doing something wrong but I can't find the Spoiler block in a P2 — it's not in the block inserter. Is it possible we don't have it on P2s at all?

@simison
Copy link
Member

simison commented Jan 14, 2022

It does show up for me in P2:

image

It's not a block that's shipped for WP.com/Jetpack customers either. :-)

Can you see the other blocks listed here?

Testing instructions at #57472 (comment)

Copy link
Contributor

@lezama lezama left a comment

Choose a reason for hiding this comment

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

this block needs a better name and icon 😅

@juanmaguitar juanmaguitar requested a review from a8ci18n January 20, 2022 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants