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

Jump to bottom

Add body class for the theme variation #7034

Merged
merged 5 commits into from
May 2, 2023
Merged

Add body class for the theme variation #7034

merged 5 commits into from
May 2, 2023

Conversation

merkushin
Copy link
Member

@merkushin merkushin commented Apr 22, 2023 •
edited
Loading

As there is no reliable way to determine the variation on backend, we save the variation in an option at the moment it the global styles are saved.

Changes proposed in this Pull Request:

  • Save theme variation on global styles update.
  • Add body class for the theme variation.

Testing instructions:

  • Go to Appearance -> Themes and activate the Course theme.
  • Go to Appearance -> Editor -> Templates -> Lesson (Learning Mode - Default) (Not really important which template to select).
  • Click Styles -> Browse Styles, select one, click Save.
  • Go to front end and check the <body> tag has a class with the name is-{lowercased-variation-title}.
  • Go to the Editor and change variation to another one. Save.
  • Go to the front end and check that the body tag variation class has updated.
  • Go to the Editor and reset global styles to defaults.
  • Check the body tag has is-default CSS class.

Related Issues:

@merkushin merkushin requested a review from a team April 22, 2023 20:31
@merkushin merkushin self-assigned this Apr 22, 2023
@carolinan
Copy link

I really wish this could be fixed in Gutenberg / core.

@mikachan
Copy link
Member

Dropping a link to a related Gutenberg issue: WordPress/gutenberg#43405

@merkushin merkushin mentioned this pull request May 1, 2023
Closed
donnapep
donnapep reviewed May 1, 2023
View reviewed changes
course/functions.php Outdated Show resolved Hide resolved
@donnapep
Copy link
Contributor

donnapep commented May 1, 2023 •
edited
Loading

@merkushin Are there any testing instructions for this PR? In particular, when is it expected that course_delete_global_styles will fire? When I tried switching to another theme, the option wasn't cleaned up. Should we consider doing that?

Also, is it a fair statement to say that existing installs of the Course theme won't have the new styles applied that use this CSS selector until they save something in the site editor?

@merkushin
Copy link
Member Author

merkushin commented May 2, 2023 •
edited
Loading

@donnapep I added testing instruction to the description, thank you.

In particular, when is it expected that course_delete_global_styles will fire?

My mistake: it won't be fired. The post isn't deleted even if we delete the theme. I'll remove it from code to avoid confusion.

When I tried switching to another theme, the option wasn't cleaned up. Should we consider doing that?

I don't think so. If the user switch back to the Course theme later, all their style changes remain the same as before switching the theme. WP keeps a wp_global_styles post for each theme.

Also, is it a fair statement to say that existing installs of the Course theme won't have the new styles applied that use this CSS selector until they save something in the site editor?

That's true. I think we can try to detect the variation. For users without custom changes it will work. However, in case they have some, we can't reliably detect the variation.

Not sure if it is worth doing, so I don't do these changes now. If we decide to do it, it might worth to do in a separate PR.

m1r0
m1r0 reviewed May 2, 2023
View reviewed changes
course/functions.php Outdated Show resolved Hide resolved
donnapep
donnapep approved these changes May 2, 2023
View reviewed changes
Copy link
Contributor

@donnapep donnapep left a comment

Choose a reason for hiding this comment

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

Works well!

@m1r0 m1r0 merged commit 679c457 into trunk May 2, 2023
@m1r0 m1r0 deleted the add/variation-body-class branch May 2, 2023 18:24
@donnapep donnapep modified the milestone: Course - 1.3.0 May 12, 2023
@merkushin merkushin mentioned this pull request May 30, 2023
Closed
@donnapep donnapep removed this from the Course - 1.3.0 milestone Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m1r0 m1r0 m1r0 left review comments

@donnapep donnapep donnapep approved these changes

Assignees

@merkushin merkushin

Labels
[Theme] Course
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@merkushin @carolinan @mikachan @donnapep @m1r0