-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Themes: Fix the archives block frontend/editor parity #30141
Themes: Fix the archives block frontend/editor parity #30141
Conversation
@@ -81,3 +81,34 @@ | |||
.items-justified-space-between { | |||
justify-content: space-between; | |||
} | |||
|
|||
.screen-reader-text { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aristath, I think you propose to add the same styles on the frontend in one of your PRs. It raised the question of whether it should be added directly to WordPress core. So far it existed only in WP-Admin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the skip link PR, theme authors can opt in to use a default style for the skip link, or they can use their own.
https://github.com/WordPress/gutenberg/pull/28946/files#diff-d8826d0f767301135f001ac1a09905bf32198b6604bf6a3fdf15799ea4c7ea48R205
The skip link is slightly different than screen-reader-text. Screen-reader-text can be, but is not always visible on focus, the skip link is always visible on focus.
Size Change: +963 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
Big fan, I look forward to being able to remove this from my own theme. I left some comments on the styling in https://github.com/WordPress/gutenberg/pull/30141/files#r600242830. Note that I'm seeing some issues in the editor with the dropdown option: That's with both toggles: Seeing it also in TT1 blocks: |
@jasmussen I'm not able to reproduce the multiple arrows issue you're seeing 🤔 |
I am not able to reproduce the problem with the arrows in the select list. The screen reader focus style is neutral, but it does not match the style from core. Empty theme I would like the select list to have a more usable default style. All new or updated code released in WordPress must conform with the WCAG 2.1 guidelines at level AA. |
I don't think it's the purpose of this PR to update the style of the default select used in the block. This just makes it match between the frontend and backend and it just relies on the browser defaults which I assume are meant to be accessible |
ec5e855
to
c38dfed
Compare
I restored the styles to match Core. |
c38dfed
to
8619887
Compare
8619887
to
1c9678b
Compare
@scruffian for the padding not applied in the same way for the archives list, it's because the markup is not the same, in the editor there's a few addition wrapping divs. Unfortunately, I can't fix that in this PR, we need to refactor the block to avoid For the dropdown, I'm unable to reproduce that issue. It looks like you and @jasmussen both had it though, so I'd appreciate if one of you can check what "revert" style is missing there. |
The problem isn't with the rules themselves but the order in which they are loaded. For me the editor styles reset gets loaded before the forms.css file from wp-admin. Since the CSS is of the same specificity, the wp-admin ones win. I added a commit which resolves this, by doubling the .editor-style-wrapper selector. I don't like it, but I haven't been able to think of another way of strengthening the CSS for just the selects. |
I think we should fix the loading order instead, we can do so by adding a dependency to the reset stylesheet. |
856b44d
to
9229c8b
Compare
In the last commit, I tried to make the stylesheet loading order deterministic, let me know if it works properly for you as I'm not able to reproduce the breakage personally. cc @jasmussen @scruffian |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes it for me 🚢
@@ -370,7 +370,7 @@ function gutenberg_register_packages_styles( $styles ) { | |||
$styles, | |||
'wp-reset-editor-styles', | |||
gutenberg_url( 'build/block-library/reset.css' ), | |||
array(), | |||
array( 'common', 'forms' ), // Make sure the reset is loaded after the default WP Adminn styles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dependency causes the admin styles to be added within the iframe. Any way we can avoid that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is adding these styles on the front-end? Surely not common.css
? Can we use the same stylesheet as the front-end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we added these dependencies here is because the "reset" style need to be loaded after "common" and "forms" otherwise you get this kind of issues #30141 (comment)
--
I'm not really sure how this relates to the iframe though because all these three stylesheets (reset, common and forms) are probably better off of the iframe (the reset style is only needed because of WP-Admin default styles).
I'm also not sure about your question about frontend, this is unrelated to frontend but in order for the editor to match frontend we need to "reset" some WP-Admin styles inside the canvas (in non-iframe mode) and that's what this reset style does.
Related #29976
The Archives block uses
.screen-reader-text
className to hide labels visually in the frontend. This PR ensures that classname is styled by default in the frontend like it should be. (in dropdown mode)This block also relies on "select" element which in wp-admin has some opinionated styles, this PR resets those styles in the editor canvas. (in dropdown mode)
This PR needs to be tested properly (specially the editor UI to ensure the select reset is not affecting anything that shouldn't).