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

Activating Gutenberg messed backend styling #6235

Closed
milindmore22 opened this issue May 14, 2021 · 26 comments · Fixed by #6570
Closed

Activating Gutenberg messed backend styling #6235

milindmore22 opened this issue May 14, 2021 · 26 comments · Fixed by #6570
Assignees
Labels
Bug Something isn't working
Projects
Milestone

Comments

@milindmore22
Copy link
Collaborator

Bug Description

Activating Gutenberg messed AMP settings page styling a little bit

Expected Behaviour

Activating any plugin should not have effects on settings

Steps to reproduce

  1. Install and Activate AMP and Gutenberg plugin
  2. Visit the AMP settings page
  3. See Template Mode toggle.

Screenshots

image

Additional context

  • WordPress version: 5.7.2
  • Plugin version: 2.1.1
  • Gutenberg plugin version (if applicable): 10.6.0
  • AMP plugin template mode: Transtional Mode
  • PHP version: 7.4
  • OS: Mac Os
  • Browser: Chrome
  • Device: Macbook Air

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@milindmore22 milindmore22 added the Bug Something isn't working label May 14, 2021
@westonruter westonruter added this to the v2.1.2 milestone May 14, 2021
@westonruter westonruter added this to Backlog in Ongoing via automation May 14, 2021
@westonruter westonruter moved this from Backlog to To Do in Ongoing May 14, 2021
@westonruter
Copy link
Member

Thanks. I saw this too but I wasn't sure if it was a problem with my Gutenberg build or it was a new issue.

@pierlon
Copy link
Contributor

pierlon commented May 14, 2021

Might be because we're bundling the CSS from @wordpress/components for the settings page. Investigating...

@pierlon pierlon self-assigned this May 14, 2021
@pierlon pierlon moved this from To Do to In Progress in Ongoing May 14, 2021
@westonruter
Copy link
Member

There is a difference in styling is due to a missing CSS rule in Gutenberg (gutenberg/build/components/style.css) which is present in core (wp-includes/css/dist/components/style.css):

image

@pierlon
Copy link
Contributor

pierlon commented May 17, 2021

Currently working on migrating used styles from wp-components into the plugin so we don't need to rely on it. If that's the only style rule missing we could add it and go ahead with that in the interim.

@westonruter
Copy link
Member

westonruter commented May 17, 2021

Present in Gutenberg 10.5.4: https://github.com/WordPress/gutenberg/blob/v10.5.4/packages/components/src/visually-hidden/style.scss
And present in 10.6.1: https://github.com/WordPress/gutenberg/blob/v10.6.1/packages/components/src/visually-hidden/style.scss
But this file is missing in v10.6.0. Nevertheless, the styles are still broken in v10.6.1.

It looks like this problem was introduced in WordPress/gutenberg#31244.

@pierlon
Copy link
Contributor

pierlon commented May 18, 2021

It looks like this problem was introduced in WordPress/gutenberg#31244.

That PR has just been reverted via WordPress/gutenberg#31882. I'll continue the work on migrating the styles into the appropriate Gutenberg components but I think this issue can be punted to the next release for now.

@pierlon
Copy link
Contributor

pierlon commented May 18, 2021

I can confirm that the Settings screen is back to normal for now (when on 67f2cf8 and WordPress/gutenberg@b5f6955):

image

@westonruter westonruter modified the milestones: v2.1.2, v2.2 May 18, 2021
@westonruter
Copy link
Member

Confirmed. Fixed in Gutenberg v10.6.1.

@westonruter
Copy link
Member

@pierlon this is being fixed in what you're doing to fix #6490?

@pierlon
Copy link
Contributor

pierlon commented Jul 29, 2021

Oh yes that would resolve this issue as well. I'll create a separate PR with those changes then as they've become mutually exclusive to the actual issue I was fixing.

@westonruter
Copy link
Member

This issue has arisen again when Gutenberg 11.3.0 is active:

image

@westonruter
Copy link
Member

@pierlon Is there a quick fix that would address this for 2.1.4?

@pierlon
Copy link
Contributor

pierlon commented Aug 26, 2021

Just created #6570 as a fix.

@pierlon pierlon modified the milestones: v2.2, v2.1.4 Aug 26, 2021
@pierlon pierlon moved this from In Progress to Ready for Review in Ongoing Aug 26, 2021
@westonruter westonruter moved this from Ready for Review to Ready for QA in Ongoing Aug 27, 2021
@westonruter
Copy link
Member

My cherry-pick (d4d2a7b) onto the 2.1 branch failed: https://github.com/ampproject/amp-wp/runs/3444490820#step:7:109

It appears due to the Gutenberg packages not being updated.

@pierlon Is this something that can be easily backported to 2.1.x? Or is it too much of a headache?

@westonruter westonruter reopened this Aug 27, 2021
@pierlon
Copy link
Contributor

pierlon commented Aug 27, 2021

It seems all that's needed is to update the JS test snapshots to match the version of the VisuallyHidden component in that branch.

@westonruter westonruter moved this from In Progress to Ready for QA in Ongoing Aug 27, 2021
@westonruter
Copy link
Member

Great! Fixed as of #6575.

@westonruter westonruter self-assigned this Aug 30, 2021
@westonruter
Copy link
Member

Humm. With the latest Gutenberg active, I'm seeing the same issue still on 2.1.4-alpha:

image

@pierlon
Copy link
Contributor

pierlon commented Aug 30, 2021

Can confirm, looking into this.

@westonruter
Copy link
Member

The issue is not present on 2.2-alpha:

image

2.1.4-alpha 2.2-alpha
Screen Shot 2021-08-30 at 14 47 55 Screen Shot 2021-08-30 at 14 52 43

@westonruter westonruter reopened this Aug 30, 2021
Ongoing automation moved this from Ready for QA to In Progress Aug 30, 2021
@westonruter westonruter assigned pierlon and unassigned westonruter Aug 30, 2021
@westonruter
Copy link
Member

westonruter commented Aug 30, 2021

I just noticed this issue is also present in the Reader theme carousel pagination:

image

@pierlon
Copy link
Contributor

pierlon commented Aug 30, 2021

The version of the @wordpress/components package on the 2.1 branch does not include the styles for the VisuallyHidden component, so I'm working on updating the package to the latest stable version. That will also mean updating all the other Gutenberg packages to ensure compatibility between the packages.

@pierlon pierlon moved this from In Progress to Ready for Review in Ongoing Aug 31, 2021
@westonruter
Copy link
Member

Fixed for 2.1.4 via #6585. Ready for QA.

@westonruter westonruter moved this from Ready for Review to Ready for QA in Ongoing Aug 31, 2021
@westonruter westonruter assigned westonruter and unassigned pierlon Aug 31, 2021
@westonruter
Copy link
Member

✅ WordPress 5.6.4 w/ Gutenberg

Settings Screen Onboarding Wizard Post Editor
image image image

✅ WordPress 5.7.2 w/ Gutenberg

Settings Screen Onboarding Wizard Post Editor
image image image

✅ WordPress 5.8 w/ Gutenberg

Settings Screen Onboarding Wizard Post Editor
image image image

@westonruter
Copy link
Member

✅ WordPress 5.6.4 w/o Gutenberg

Settings Screen Onboarding Wizard Post Editor
image image image

✅ WordPress 5.8 w/o Gutenberg

Settings Screen Onboarding Wizard Post Editor
image image image

@westonruter
Copy link
Member

I've tested various screens in the different versions and everything seems to check out.

@delawski Maybe you have some other areas in mind to test as well. Otherwise, I'm moving this to QA Passed.

@westonruter westonruter moved this from Ready for QA to QA Passed in Ongoing Aug 31, 2021
@delawski
Copy link
Collaborator

@delawski Maybe you have some other areas in mind to test as well. Otherwise, I'm moving this to QA Passed.

All good here 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Ongoing
  
QA Passed
Development

Successfully merging a pull request may close this issue.

4 participants