sensei
sensei copied to clipboard
Update font sizes for module title and lesson header
Resolves https://github.com/Automattic/themes/issues/7085
Proposed Changes
- Updates font size of lesson header and Module title in Course theme
We've used the preset font sizes of course theme. We've used the medium font size for the module name and small font size for the lesson title.
For default variation of Course theme, we didn't change the font size as the medium of default was a bit smaller.
Testing Instructions
- Checkout this branch of Course theme https://github.com/Automattic/themes/pull/7465
- Create a Course, in Course Outline block, add a few modules and lessons
- Make sure the Lesson title looks bigger
- Make sure the Module name is bigger than Lesson title
- Check for all variations of the Course theme
- Check with other themes to make sure nothing has broken with them.
Screenshots:
Pre-Merge Checklist
- [x] PR title and description contain sufficient detail and accurately describe the changes
- [x] Acceptance criteria is met
- [ ] Decisions are publicly documented
- [x] Adheres to coding standards (PHP, JavaScript, CSS, HTML)
- [x] All strings are translatable (without concatenation, handles plurals)
- [x] Follows our naming conventions (P6rkRX-4oA-p2)
- [ ] Hooks (p6rkRX-1uS-p2) and functions are documented
- [x] New UIs are responsive and use a mobile-first approach
- [ ] New UIs match the designs
- [ ] Different user privileges (admin, teacher, subscriber) are tested as appropriate
- [ ] Code is tested on the minimum supported PHP and WordPress versions
- [ ] User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
- [ ] "Needs Documentation" label is added if this change requires updates to documentation
- [ ] Known issues are created as new GitHub issues
WordPress Dependencies Report
The github-action-wordpress-dependencies-report
action has detected some script changes between the commit 83e8edacedc39888d935ef3dafb03140c0cc3dc9 and trunk. Please review and confirm the following are correct before merging.
Script Handle | Added Dependencies | Removed Dependencies | Total Size | Size Diff |
---|---|---|---|---|
css/3rd-party/themes/course/style.js |
wp-polyfill |
24 B | +24 B ( +100% πΌ ) |
This comment was automatically generated by the github-action-wordpress-dependencies-report
action.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 51.93%. Comparing base (
2c5b426
) to head (83e8eda
). Report is 1 commits behind head on trunk.
Additional details and impacted files
@@ Coverage Diff @@
## trunk #7068 +/- ##
=========================================
Coverage 51.93% 51.93%
Complexity 11277 11277
=========================================
Files 631 631
Lines 47710 47710
Branches 421 421
=========================================
Hits 24779 24779
Misses 22594 22594
Partials 337 337
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Ξ = absolute <relative> (impact)
,ΓΈ = not affected
,? = missing data
Powered by Codecov. Last update 28db587...83e8eda. Read the comment docs.
@Imran92 I'm not sure that this should be configurable via a block setting for a few reasons:
- It's a bit weird that you can set the Lessons subheading font size but not the module font size.
- It makes it possible to set the Lessons subheading font size to be larger than the module font size, which we probably don't want to enable since the module is an
<h2>
and the Lessons subheading is an<h3>
. - Changes to the UI should really go through design first for feedback.
There are already some notes in the issue description where Andrei left some design feedback:
We could easily bump it up to 20px, and increase the padding top & bottom to 10px, instead of 5 right now.
I added a consideration as well:
We should be careful not to make the "Lessons" text bigger than the name of the module. For example, the module name for the Blue variation has a font size of 19.8px, so changing "Lessons" text to 20px would make it larger than the module, which is not what we want. Given this, we'll likely need to bump the font size of the module proportionally as well.
Another thought that may be better - We could obey the heading font sizes that are in the original designs. So the module (<h2>
)nwould be 48px
and Lessons subheading (<h3>
) would be 36px
on desktop.
If you like, we could move this one back to To Do given there are higher priority things now. π
Thanks for taking a look @donnapep π
Another thought that may be better - We could obey the heading font sizes that are in the nf4oV51c1JBxJKXshMe7cd-fi-20020042%3A412. So the module (
<h2>
)nwould be48px
and Lessons subheading (<h3>
) would be36px
on desktop.
I tried using the default sizes of h2 and h3 (48px and 36px), it looked a bit too big to me. I'm attaching two screenshots below, let me know what you think
We could easily bump it up to 20px, and increase the padding top & bottom to 10px, instead of 5 right now.
I went for this approach. basically I'm using the preset font sizes of Course theme now. I'm using the medium size for the module name and small for the lesson title, in default variation, small is 20px. I've also increased the padding to 10px.
We should be careful not to make the "Lessons" text bigger than the name of the module. For example, the module name for the Blue variation has a font size of 19.8px, so changing "Lessons" text to 20px would make it larger than the module, which is not what we want. Given this, we'll likely need to bump the font size of the module proportionally as well.
As we are using the medium and small presets now, that takes care of this issue as small is smaller than medium.
Could we change the default font size for modules from
1.1em
to20px
, and "Lessons" from11px
to14px
? It's a bit too small on other themes.
Good suggestion, I've updated them here https://github.com/Automattic/sensei/pull/7068/commits/d3e9c3a4b7e7a81d2f291df9c53688f645578131. I've used rem units for the font sizes.
Test the previous changes of this PR with WordPress Playground.