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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 31, 2021
Merged

Themes: Fix the archives block frontend/editor parity #30141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d80f954
Fix the archives block frontend/editor parity
youknowriad Mar 23, 2021
1c9678b
Fix lists box-sizing
youknowriad Mar 29, 2021
9229c8b
deterministic style loading order
youknowriad Mar 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/client-assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

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?

Copy link
Member

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?

Copy link
Contributor Author

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.

$version
);
$styles->add_data( 'wp-reset-editor-styles', 'rtl', 'replace' );
Expand Down
31 changes: 31 additions & 0 deletions packages/block-library/src/common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,34 @@
.items-justified-space-between {
justify-content: space-between;
}

.screen-reader-text {
Copy link
Member

@gziolo gziolo Mar 23, 2021 •
edited
Loading

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.

Copy link
Contributor

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.

border: 0;
clip: rect(1px, 1px, 1px, 1px);
-webkit-clip-path: inset(50%);
clip-path: inset(50%);
height: 1px;
margin: -1px;
overflow: hidden;
padding: 0;
position: absolute;
width: 1px;
word-wrap: normal !important;
}

.screen-reader-text:focus {
background-color: $gray-300;
clip: auto !important;
clip-path: none;
color: #444;
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
display: block;
font-size: 1em;
height: auto;
left: 5px;
line-height: normal;
padding: 15px 23px 14px;
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
text-decoration: none;
top: 5px;
width: auto;
z-index: 100000;
}
43 changes: 35 additions & 8 deletions packages/block-library/src/reset.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
ol {
margin: revert;
padding: revert;
list-style-type: revert;
box-sizing: revert;

// Remove bottom margin from nested lists.
ul,
Expand All @@ -73,14 +75,6 @@
}
}

ul {
list-style-type: revert;
}

ol {
list-style-type: revert;
}

ul ul,
ol ul {
list-style-type: revert;
Expand All @@ -98,4 +92,37 @@
line-height: revert;
font-weight: revert;
}

select {
font-family: system-ui;
-webkit-appearance: revert;
color: revert;
border: revert;
border-radius: revert;
background: revert;
box-shadow: revert;
text-shadow: revert;
outline: revert;
cursor: revert;
transform: revert;
font-size: revert;
line-height: revert;
padding: revert;
margin: revert;
min-height: revert;
max-width: revert;
vertical-align: revert;
font-weight: revert;
}

select:disabled,
select:focus {
color: revert;
border-color: revert;
background: revert;
box-shadow: revert;
text-shadow: revert;
cursor: revert;
transform: revert;
}
}