Skip to content
This repository has been archived by the owner on Feb 17, 2025. It is now read-only.

Add heading rules to varia based themes #2761

Closed
wants to merge 3 commits into from
Closed

Conversation

scruffian
Copy link
Member

This is a different approach to #2551 which fixes all Varia based themes.

Comment on lines 730 to 732
.wp-block-group {
/* Treating H2 separately to account for legacy /core styles */
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we getting this empty rule because of the comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

seems like it. interestingly switching to a // comment removes it...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think the Sass compiler completely removes // but it keeps /* … */.

I believe there’s a separate, later script on wpcom that minifies each theme stylesheet which removes all code comments in CSS.

@MaggieCabrera
Copy link
Contributor

I was trying to review but I'm not sure what the correct behavior is. A heading with no defined alignment goes to the right on RTL site, the rest goes to the alignment that was selected in the editor, is that correct? If so, then this looks fine to me:

Screenshot 2020-11-03 at 18 38 17

@scruffian
Copy link
Member Author

Looking at this some more I think it's a bad idea, see #2551 (comment)

@scruffian scruffian closed this Nov 3, 2020
@scruffian scruffian deleted the fix/varia-rtl branch November 3, 2020 23:03
@allancole
Copy link
Contributor

This is what I’m getting on my end with RTL turned on inside of a group block:

image

Looks like the alignment is still getting reversed in RTL, and I think its supposed to stay persistent regardless of the RTL setting.

image

For other text-align utility class rules we’re using a Sass flag to ignore the RTL so the floating position isn’t automatically flipped, which breaks the expected behavior. See here:

.alignleft {
/*rtl:ignore*/
text-align: left;
/*rtl:ignore*/
float: left;

@scruffian
Copy link
Member Author

I wonder if we can just remove these rules and let core handle it all?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants