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

Copy/Paste Styles can give a Block a 'style' that it isn't allowed to have #60352

Open
MadtownLems opened this issue Apr 1, 2024 · 9 comments
Labels
[Feature] Block Style Variations Issues or PRs that are related to the style variations for blocks [Type] Bug An existing feature does not function as intended

Comments

@MadtownLems
Copy link

Description

When you "Copy Styles" and "Paste Styles" from one block to another, you can give the destination block a style it shouldn't support. For example, with TT4, you can make a Paragraph be "With Asterisk"

Step-by-step reproduction instructions

Using TT4:

  1. Create a Post with a Heading and a Paragraph Block.
  2. Give the Heading block the style "With Asterisk"
  3. "Copy Styles" on the Heading
  4. "Paste Styles" to the Paragraph
  5. See that the Paragraph now has the style "With Asterisk", even though there's no UI for it.

Screenshots, screen recording, code snippet

styles

Environment info

6.4.3 and 6.5-RC-4.
With and without latest Gutenberg plugin.

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@MadtownLems MadtownLems added the [Type] Bug An existing feature does not function as intended label Apr 1, 2024
@ndiego
Copy link
Member

ndiego commented Apr 2, 2024

The "Copy styles" option also copies any custom CSS classes and the asterisk is added by the .is-style-asterisk class. Under the hood, block styles are just classes.

So I am not sure if this is a bug. Perhaps the .is-style-asterisk class should have been scoped to headings in TT4. I could see a scenario where I have added a few custom utility classes to a block and then it would be nice if those get copied over.

Any thoughts on this @jameskoster or @kevin940726? I believe you both were involved in the original implementation.

@jordesign jordesign added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Block Style Variations Issues or PRs that are related to the style variations for blocks and removed Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Apr 2, 2024
@t-hamano
Copy link
Contributor

t-hamano commented Apr 2, 2024

I think this issue should be fixed on the TT4 theme side, and the block name should be included in the inline style selector.

The following block styles, currently defined in this section, affect all blocks.

  • is-style-arrow-icon-details
  • is-style-pill
  • is-style-asterisk

I would like to file a core ticket regarding this issue.

@t-hamano
Copy link
Contributor

t-hamano commented Apr 2, 2024

@jameskoster
Copy link
Contributor

jameskoster commented Apr 2, 2024

The "Copy styles" option also copies any custom CSS classes

I think this is a good point to discuss.

I don't have a strong feeling, but anecdotally the current behavior feels a bit unexpected to me. I would anticipate 'Copy styles' to only capture styles added via design tools in the Inspector UI. Applying a custom css class to another block feels like something to do manually.

Is the current behavior definitely what we want?

@annezazu
Copy link
Contributor

annezazu commented Apr 2, 2024

Agreed -- I wouldn't expect a custom CSS class to be moved over and would want it to only snag what's in the design tools. Curious to hear from others though.

@ndiego
Copy link
Member

ndiego commented Apr 2, 2024

I wouldn't expect a custom CSS class to be moved over and would want it to only snag what's in the design tools.

That is definitely the more expected approach, and I agree with it. Just curious if the CSS classes were explicitly included for some reason. Seems like this was intentional at one point.

@t-hamano
Copy link
Contributor

t-hamano commented Apr 3, 2024

If we want to avoid copying custom CSS classes, we can probably just remove this line.

The ability to paste styles was added in #45477, but this comment also seems to mention class names.

@kevin940726, Do you know what the possible problems are if we exclude CSS classes from pasting styles?

@MadtownLems
Copy link
Author

MadtownLems commented Apr 3, 2024 via email

@t-hamano
Copy link
Contributor

When a block style is selected, the class name "is-style-{slug} is added to the additional CSS class, but it is difficult to determine if this is related to the block style or if it was added intentionally by the user.

There are several possible approaches, do you have any ideas?

  1. Copy all additional CSS classes (Current specification): This may be useful if the user uses a common class name for all blocks and wants to apply that class name to the pasted block as well. However, as this issue originally raised, unintended styles may be applied to the block being pasted.
  2. Don't copy all additional CSS classes: This prevents the problem originally raised by this issue, but may be inconvenient if users use common class names for all blocks.
  3. Conditionally copy additional CSS classes: This is probably the most complex. Perhaps we need a rule like this:
    • Scans the copied block for additional CSS classes and extracts additional CSS classes with the is-style- prefix.
    • If the pasted block does not have a style that matches that CSS class, remove only that CSS class and inherit the CSS classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Style Variations Issues or PRs that are related to the style variations for blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

6 participants