fix: unit page title no longer overlaps the Live/Discussions badges - #3243
farhaanbukhsh merged 1 commit into
Conversation
|
Thanks for the pull request, @AhtishamShahid! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3243 +/- ##
=======================================
Coverage 96.02% 96.02%
=======================================
Files 1401 1401
Lines 34129 34129
Branches 7842 7842
=======================================
Hits 32772 32772
Misses 1316 1316
Partials 41 41 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The unit page cancelled the blank space the shared SubHeader reserves below
its action buttons with `.course-unit .sub-header { margin-bottom: -35px }`.
That shifts everything after the sub header up by a fixed amount regardless of
how tall the title is, which is only correct while the unit display name fits
on a single line. `.sub-header` is a flex row, so once the `<h2>` grows taller
than the action buttons the negative margin eats into the title instead of the
reserved gap, and the "Live" / "Discussions Enabled" status bar is drawn on top
of the last line of the display name.
Remove the reserved space at its source instead, by zeroing the
`.sub-header-actions` bottom margin within `.course-unit`. The correction is
then proportional to the actual header height, so the status bar always lands
below the title no matter how many lines it wraps to. Scoped to `.course-unit`,
so the shared SubHeader on other pages is unaffected.
2a77188 to
0940e69
Compare
There was a problem hiding this comment.
👍
✅ I tested this:
- by creating a big unit name
- the name overlaps on master
- the name is rendered properly on the feature branch.
- played around with the css and the solution looked optimized
✅ I read through the code
✅ I checked for accessibility issues
❌ Includes documentation
|
Thank you @AhtishamShahid for the fix :) |
Description
To hide blank space under the sub header's action buttons, the unit page pulled everything below the sub header up by a fixed 35px (
.course-unit .sub-header { margin-bottom: -35px }). Once the unit display name wraps to a second line the title, not the buttons, sets the header's height, so those 35px come out of the title and the "Live" / "Discussions Enabled" badges are painted over it.This zeroes the
.sub-header-actionsbottom margin inside.course-unitinstead, closing the same gap by the right amount at any title height. Scoped to.course-unit, so the sharedSubHeaderon other pages is untouched.Before
1000px-wide window, display name wrapping to three lines:
After
Supporting information
Reported as
mitodl/hq#12844(MIT Open Learning, private tracker).Testing instructions
openedxdiscussions plusdiscussion_enabledon the unit (Discussions Enabled badge).Other information
openedx-authoring-dev:22.0.0) at 1000px (three-line title), 1500px (two-line title), and with a one-line title.npx stylelint src/course-unit/CourseUnit.scsspasses; the Jest suite was not run locally.release/verawoodin fix: unit page title no longer overlaps the Live/Discussions badges [backport to verawood] #3244 as the same commit.Best Practices Checklist
CSS-only change: no new files, components, state, API calls, or translated strings, so none of the checklist items apply.