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

Block Styles: Truncate long button labels #42975

Merged
merged 3 commits into from
Aug 9, 2022

Conversation

shimotmk
Copy link
Contributor

@shimotmk shimotmk commented Aug 4, 2022

Resolves #40463
Related #40807 #41455

What?

Changed character limit to be calculated from character width.

Why?

How?

#40463 (comment) is used to calculate the width of the character count using the string-width package.
#40463 (comment)

Before

before

After

after

Testing Instructions

  1. Refer to the following code to add a block style.
function twenty_twenty_two_register_block_styles() {
	// Hello,Hello,…
	register_block_style(
		'core/image',
		array(
			'name'  => 'twentytwentytwo-image-1',
			'label' => esc_html__( 'Hello,Hello,Hello', 'twentytwentyone' ),
		)
	);

	// Hola,Hola,Ho…
	register_block_style(
		'core/image',
		array(
			'name'  => 'twentytwentytwo-image-2',
			'label' => esc_html__( 'Hola,Hola,Hola', 'twentytwentyone' ),
		)
	);

	// こんにちは,こんにちは
	register_block_style(
		'core/image',
		array(
			'name'  => 'twentytwentytwo-image-3',
			'label' => esc_html__( 'こんにちは,こんにちは', 'twentytwentyone' ),
		)
	);

	// 你好,你好,你好,你好
	register_block_style(
		'core/image',
		array(
			'name'  => 'twentytwentytwo-image-4',
			'label' => esc_html__( '你好,你好,你好,你好', 'twentytwentyone' ),
		)
	);

	// Hello,こんにちは
	register_block_style(
		'core/image',
		array(
			'name'  => 'twentytwentytwo-image-5',
			'label' => esc_html__( 'Hello,こんにちは', 'twentytwentyone' ),
		)
	);
}
add_action( 'init', 'twenty_twenty_two_register_block_styles' );
  1. Go to the block editor and place the image block.

  2. Open the side panel and look at the style preview.

Screenshots or screencast

Text → Truncate
@shimotmk shimotmk requested a review from ellatrix as a code owner August 4, 2022 11:20
@mirka mirka self-requested a review August 5, 2022 04:35
@mirka mirka assigned mirka and shimotmk and unassigned mirka Aug 5, 2022
@mirka mirka added [Type] Bug An existing feature does not function as intended [Package] Block editor /packages/block-editor labels Aug 5, 2022
@mirka mirka changed the title Fix blockType Title Text Block Styles: Truncate long button labels Aug 5, 2022
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

I see that the ellipsis isn't being shown. This is because the Button component has white-space: nowrap. We need to add a few more styles for this to look good (please keep the code comments, since they are non-obvious 🙂):

.block-editor-block-styles__item-text {
	// The Button component is white-space: nowrap, and that won't work with line-clamp.
	white-space: normal;

	// Without this, the ellipsis can sometimes be partially hidden by the Button padding.
	text-align: start;
	text-align-last: center;
}

The text-align is an interesting one. Notice how the ellipsis can get partially hidden without the special handling:

Before After
Without text-align trick With text-align trick

(cc @ciampo for awareness. We might want to bake this into the components library if we encounter this again.)

@shimotmk
Copy link
Contributor Author

shimotmk commented Aug 6, 2022

@mirka
Thanks for your comment.
I still need to adjust the css.

I was able to confirm that the ellipsis is displayed in long CJK characters after applying the suggested CSS.

However, the ellipsis is no longer displayed for long English words.
So I added word-break: break-all;.

block-type

@@ -43,6 +43,16 @@
display: inline-block;
width: calc(50% - #{$grid-unit-05});

&-text {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'd prefer this to be written out (.block-editor-block-styles__item-text) so it can be searched easily across the codebase.

Also, no need to nest it this deep since the class will never appear anywhere else, and it just adds unnecessary specificity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mirka
Thanks for the review.

I've adjusted the .block-editor-block-styles__item-text class to make it easier to isolate and search.

How about something like this?

@@ -43,6 +43,16 @@
display: inline-block;
width: calc(50% - #{$grid-unit-05});

&-text {
word-break: break-all;
Copy link
Member

Choose a reason for hiding this comment

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

Great catch, I looked into this a little further and it's pretty interesting. This might be something we need to bake into Truncate. Maybe word-break: break-all when numberOfLines is 1, and overflow-wrap: break-word when it's multiple lines. I'll keep track of this as a separate issue.

Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the solid i18n improvement! 🚀

@mirka mirka merged commit e6f3d8e into WordPress:trunk Aug 9, 2022
@github-actions github-actions bot added this to the Gutenberg 13.9 milestone Aug 9, 2022
@oandregal oandregal added the [Package] Components /packages/components label Aug 16, 2022
@gziolo gziolo added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 22, 2022
gziolo pushed a commit that referenced this pull request Aug 23, 2022
* fix blockType Text
Text → Truncate

* Block Styles css

* Block Styles css
@gziolo
Copy link
Member

gziolo commented Aug 23, 2022

Cherry-picked for WordPress 6.0.2 release with 693a7ac.

@gziolo gziolo removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 23, 2022
@shimotmk shimotmk deleted the fix/block-type-text branch September 2, 2022 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Block Styles preview name: Character limit is not working correctly for some languages.
4 participants