Let buttons get their styles from the theme #7587

donnapep · 2024-04-10T19:24:55Z

Resolves #7090.

Proposed Changes

frontend.css is loaded for lessons in Learning Mode, even though LM lessons uses very little of the CSS in that file. This CSS file is responsible for styling buttons, which was the reason why the comment styling looked so bad.

To resolve this, this PR stops loading frontend.css for lessons in Learning Mode. To facilitate this change, it also:

Moves CSS specific to the Lesson Properties block out of frontend.scss and into a block-specific stylesheet (lesson-properties.scss).
Moves a bit of CSS needed for the Lesson Actions buttons to single-lesson-style.scss.
No longer styles buttons anywhere (Learning Mode and not), and instead lets the theme handle it.

Note that I didn't reference the Figma designs linked in the issue as it's no longer possible to see things like spacing, colors etc. without Figma's Dev Mode. So I just tried to make it look decent.

Testing Instructions

Check out Update comments template to match latest Gutenberg block markup themes#7736.
Open the Lesson (Learning Mode - Default) template in the site editor.
Add the Comments block to the Main Content area.
Add the Lesson Properties block to the Main Content area.
Also add the Lesson Properties block to a lesson.
On the frontend, add a few comments to that same lesson.
Enable Learning Mode and the Course theme.
Check that lesson comment styling looks good.
Check that the Lesson Properties block styling looks good.
Check other themes. The styling should look decent on other themes, not perfect, but the buttons should definitely not be the blue color they were before. They likely won't match the Lesson Actions button styles.
Disable Learning Mode.
Check that lesson comment styling looks good and the comment form buttons use the theme's styling. (To easily verify this, in a new tab add some comments to a post. Disable Sensei. Compare the comment buttons of the post to the lesson. They should look the same.)
Check that the Lesson Properties block styling looks good.
Check the lesson on different themes, including Course.

Note: There may be slight differences between the comments in Learning Mode vs. those in the legacy view (i.e. non-Learning Mode) for the Course theme, as the ones in the legacy view already have some non-default block settings set for the comment-related blocks in https://github.com/Automattic/themes/blob/trunk/course/patterns/comments.php. The differences will be theme-dependent.

Pre-Merge Checklist

Screenshots

No Learning Mode, Course Theme - Before

No Learning Mode, Course Theme - After

Learning Mode, Course Theme - After

It messes up the button styles and uses very little CSS from that stylesheet otherwise.

github-actions · 2024-04-10T19:29:04Z

Test the previous changes of this PR with WordPress Playground.

github-actions · 2024-04-10T19:29:31Z

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit 0d4a268 and trunk. Please review and confirm the following are correct before merging.

Script Handle	Added Dependencies	Removed Dependencies	Total Size	Size Diff
`blocks/single-lesson-style.js`	`wp-polyfill`		24 B	+24 B ( +100% 🔼 )

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

Sensei should not be so opinionated.

github-actions · 2024-04-11T15:51:35Z

Test the previous changes of this PR with WordPress Playground.

github-actions · 2024-04-11T16:13:52Z

Test the previous changes of this PR with WordPress Playground.

merkushin · 2024-04-12T03:06:53Z

Tested it with the following themes: Course, Astra, Divi, 2021.

I noticed that in Astra without Learning Mode comments look not well. I'm attaching also a screenshot of a post page with comment where the comments look fine.

And in Divi with Learning Mode comments look also not well... And the comment submit button looks unstyled. (Screenshots below.)

Preparation:

With Learning Mode

Course Theme

Astra

Divi

2021

Without Learning Mode

Course Theme

Astra

Divi

2021

donnapep · 2024-04-12T13:33:02Z

@merkushin I think a lot of that is expected. I concentrated primarily on making the Course theme comments look good, so if there are any issues there I would definitely want to fix those. After that, I concentrated on ensuring that the button styles matched what the theme uses. So for Astra, for example, the buttons now use Astra's button styles.

One improvement I will try to make in this PR is to have the non-block themes use Learning Mode's button styles instead of the theme's, but I don't think I'll attempt to fix any other sort of spacing issues (unless they didn't exist before this PR). ~~Perhaps I'll open a separate issue for those and work on them next if it doesn't seem like it would be too much work.~~ Changed my mind. There is other work that is higher priority. 🙂

merkushin

Comments in the Course theme look good.

After that, I concentrated on ensuring that the button styles matched what the theme uses.

In Divi I noticed the submit button is unstyled at all, it uses neither Course theme styles, nor Divi styles.

But if our focus here is only on the Course theme, then it's fine.

github-actions · 2024-04-15T15:41:58Z

Test the previous changes of this PR with WordPress Playground.

donnapep · 2024-04-15T19:49:08Z

@merkushin I've updated the Post Comment button styles in Learning Mode for both classic and block themes in 0d4a268 and 3872b8a. The button should now look similar to the Next Lesson button, but without a border.

Divi

Note that I didn't attempt to style the Reply "button" as Divi is doing some fancy things there.

Twenty Twenty Four

github-actions · 2024-04-15T20:01:21Z

Test the previous changes of this PR with WordPress Playground.

github-actions · 2024-04-15T20:06:15Z

Test the previous changes of this PR with WordPress Playground.

merkushin

Thanks for the update! The button looks good everywhere now! :)

donnapep added 4 commits April 9, 2024 12:09

Don't load frontend.css for Learning Mode lessons only

19fce06

It messes up the button styles and uses very little CSS from that stylesheet otherwise.

Move Lesson Properties block CSS that is needed for LM lessons

6167754

Move some other lesson CSS to new single lesson stylesheet

f9f9608

Load lesson properties CSS in Learning Mode

91d8d61

donnapep self-assigned this Apr 10, 2024

Allow the theme to style the buttons by default

8166d88

Sensei should not be so opinionated.

donnapep added this to the 4.23.1 milestone Apr 11, 2024

donnapep changed the title ~~Fix comment styles~~ Let buttons get their styles from the theme Apr 11, 2024

Add change log entry

a0ab6c2

donnapep marked this pull request as ready for review April 11, 2024 16:08

donnapep requested a review from a team April 11, 2024 19:11

merkushin requested review from merkushin and removed request for a team April 12, 2024 03:07

merkushin previously approved these changes Apr 15, 2024

View reviewed changes

donnapep modified the milestones: 4.23.1, 4.23.2 Apr 15, 2024

Merge branch 'trunk' into fix/style-comments

0e82071

Style "Post Comment" button in Learning Mode

0d4a268

donnapep dismissed merkushin’s stale review via 0d4a268 April 15, 2024 19:43

Add some Divi-specific CSS for aligning the button

3872b8a

merkushin approved these changes Apr 16, 2024

View reviewed changes

donnapep merged commit bc6a248 into trunk Apr 16, 2024
23 checks passed

donnapep deleted the fix/style-comments branch April 16, 2024 12:07

This was referenced May 31, 2024

Contact teacher function not working properly (Learner mode) #7605

Closed

Fix contact teacher form not displaying correctly in Learning Mode #7610

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let buttons get their styles from the theme #7587

Let buttons get their styles from the theme #7587

donnapep commented Apr 10, 2024 •

edited

Loading

github-actions bot commented Apr 10, 2024

github-actions bot commented Apr 10, 2024 •

edited

Loading

github-actions bot commented Apr 11, 2024

github-actions bot commented Apr 11, 2024

merkushin commented Apr 12, 2024

donnapep commented Apr 12, 2024 •

edited

Loading

merkushin left a comment •

edited

Loading

github-actions bot commented Apr 15, 2024

donnapep commented Apr 15, 2024 •

edited

Loading

github-actions bot commented Apr 15, 2024

github-actions bot commented Apr 15, 2024

merkushin left a comment

Let buttons get their styles from the theme #7587

Let buttons get their styles from the theme #7587

Conversation

donnapep commented Apr 10, 2024 • edited Loading

Proposed Changes

Testing Instructions

Pre-Merge Checklist

Screenshots

No Learning Mode, Course Theme - Before

No Learning Mode, Course Theme - After

Learning Mode, Course Theme - After

github-actions bot commented Apr 10, 2024

github-actions bot commented Apr 10, 2024 • edited Loading

WordPress Dependencies Report

github-actions bot commented Apr 11, 2024

github-actions bot commented Apr 11, 2024

merkushin commented Apr 12, 2024

Preparation:

With Learning Mode

Course Theme

Astra

Divi

2021

Without Learning Mode

Course Theme

Astra

Divi

2021

donnapep commented Apr 12, 2024 • edited Loading

merkushin left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 15, 2024

donnapep commented Apr 15, 2024 • edited Loading

Divi

Twenty Twenty Four

github-actions bot commented Apr 15, 2024

github-actions bot commented Apr 15, 2024

merkushin left a comment

Choose a reason for hiding this comment

donnapep commented Apr 10, 2024 •

edited

Loading

github-actions bot commented Apr 10, 2024 •

edited

Loading

donnapep commented Apr 12, 2024 •

edited

Loading

merkushin left a comment •

edited

Loading

donnapep commented Apr 15, 2024 •

edited

Loading