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

Backport: Gutenberg 4434 #3331

Closed
wants to merge 6 commits into from

Conversation

scruffian
Copy link

This is an attempt at a backport of WordPress/gutenberg#44334

The main part of this change is adding a new CSS file and I'm not sure how to do this - I'm guessing this comes from the packages, rather than me adding it manually.

@ockham any insight would be appreciated.

@ockham
Copy link
Contributor

ockham commented Sep 27, 2022

This is an attempt at a backport of WordPress/gutenberg#44334

The main part of this change is adding a new CSS file and I'm not sure how to do this - I'm guessing this comes from the packages, rather than me adding it manually.

I think that's accurate; according to unpkg.com, the package currently contains files such as common.css or theme.css at root level, so I'd expect classic.css to show up there, too.

I'll be publishing a new round of npms for WP 6.1 Beta 2 later today, and since we've cherry-picked WordPress/gutenberg#44334 to be included with it, we can check unpkg later to see if the file ended up there.

The path you're using to then locate the file in Core after a sync and build also matches where I'm seeing those other CSS files locally.

(To recap, we'll need to wait for packages to be published and synced before we can land this PR 😊 )

@ockham
Copy link
Contributor

ockham commented Sep 27, 2022

@ockham
Copy link
Contributor

ockham commented Sep 27, 2022

I'll rebase so this will pick up classic.css, now that the sync PR has landed.

scruffian and others added 3 commits September 27, 2022 21:44
Co-authored-by: Bernie Reiter <ockham@raz.or.at>
Co-authored-by: Bernie Reiter <ockham@raz.or.at>
@ockham ockham force-pushed the backport/gutenberg-44334 branch from cafcbbc to e71a7b8 Compare September 27, 2022 19:45
Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

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

Thank you @scruffian!

Confirming that this enqueues classic.css both on the frontend and in the editor ✅

LGTM!

Comment on lines +3656 to +3659
// To load classic theme styles on the frontend.
add_action( 'wp_enqueue_scripts', 'wp_enqueue_classic_theme_styles' );
// To load classic theme styles in the the editor.
add_action( 'admin_enqueue_scripts', 'wp_enqueue_classic_theme_styles' );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these should probably be moved to wp-includes/default-filters.php where all other core hooks live. Probably around https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/default-filters.php#L558

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll handle this in the merge though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @dream-encode!

@ockham
Copy link
Contributor

ockham commented Sep 29, 2022

Some more detail from WordPress/gutenberg#44334 paper trail:

WordPress/gutenberg#43981 has testing instructions:

Testing Instructions

  1. Switch to a classic theme that doesn't specify its own button styles (I'm using Canape)
  2. Check that button look broken in trunk
  3. Check out this PR
  4. Check that buttons now look correct:
Screenshot 2022-09-08 at 14 21 57

The styling was apparently originally removed by WordPress/gutenberg#34180, which was merged into Gutenberg in June. This means that it likely made its way into Core with the first package sync for WP 6.1. Testing this now.

@ockham
Copy link
Contributor

ockham commented Sep 29, 2022

The styling was apparently originally removed by WordPress/gutenberg#34180, which was merged into Gutenberg in June. This means that it likely made its way into Core with the first package sync for WP 6.1. Testing this now.

Confirming:

Before 3b63a75 At 3b63a75
image image

@dream-encode
Copy link
Contributor

Merged into core in https://core.trac.wordpress.org/changeset/54358.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants