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

Inserter: Encapsulate styles for tablist and close button #61760

Merged

Conversation

mattsherman
Copy link
Contributor

What?

This PR moves the CSS for the tabllist and close button layout into the inserter itself.

Why?

In #61421, the close button was added, and the CSS styling was added to the InserterSidebar instead of the Inserter itself.

This broke the styling for consumers using the Inserter directly (or __experimentalLibrary), such as the new product editor in WooCommerce (see woocommerce/woocommerce#47561).

How?

The CSS is moved to the Inserter itself.

In addition, the CSS class names are changed to be more accurate to what they actually are, which will help with maintainability and also allow for a temporary workaround to be applied by those using Inserter/__experimentalLibrary directly while allowing them to automatically get future styling changes without having to remove the workaround.

Testing Instructions

  1. Open the inserter
  2. Verify styling is correct.

(Testing instructions for WooCommerce specifically will be part of woocommerce/woocommerce#47561)

Screenshots or screencast

Broken (in WooCommerce)

Screenshot 2024-05-17 at 07 07 04

Fixed (no change in core Gutenberg styling)

Screenshot 2024-05-17 at 07 08 04

Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: .

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

1 similar comment
Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: .

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

Copy link

github-actions bot commented May 17, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mattsherman <mattormeeple@git.wordpress.org>
Co-authored-by: retrofox <retrofox@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@retrofox retrofox left a comment

Choose a reason for hiding this comment

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

LGTM.

It fixes the broken styles between old versions:

Product Editor - GB trunk Product Editor - This PR
Screenshot 2024-05-21 at 13 01 14 Screenshot 2024-05-21 at 13 21 05

@mattsherman mattsherman force-pushed the fix/inserter-styles-not-encapsulated branch from baf6c3e to 29f2b25 Compare May 24, 2024 19:41
@mattsherman
Copy link
Contributor Author

@retrofox There was a conflict in one of the files, so I pushed an update to resolve the conflict. Thanks for the initial review. Can you take another look at this when you get a chance?

Also, is there a way to re-run CI checks? I don't seem to be able to. The "Compressed Size / Check (pull_request)" is failed, but I don't think it is due to my changes.

@scruffian This PR moves some CSS that you recently touched, so I'd love to have your input as well, to make sure my change makes sense in the context of the work you've been doing. Thanks!

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

This code LGTM!

@retrofox retrofox merged commit 18ca36c into WordPress:trunk May 28, 2024
61 of 62 checks passed
@github-actions github-actions bot added this to the Gutenberg 18.5 milestone May 28, 2024
@mattsherman mattsherman deleted the fix/inserter-styles-not-encapsulated branch May 28, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants