fix(layout): increase gap between sidebar and content #1154
+286
−317
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add --layout-sidebar-padding variable for padding between sidebar content and document content, so the scrollbar isn't pushed up against the content this removes my ugly hack from before where I set a negative right margin on the left sidebar, which caused it to overlay the heading anchor links sets --layout-sidebar-padding to 1rem to make the gap between sidebar content and document content 3rem increases --layout-sidebar-min by 1rem so sidebars content doesn't get any narrower absorbs the --layout-side-padding-min padding into the left of the left sidebar to avoid 2px padding for focus outlines and give us greater flexibility with the sidebar layout in the future does the same on the right of the right sidebar to align the right sidebar scrollbar with the right of the screen on smaller displays
move them up to .reference-layout__sidebar
…from 2-sidebars committed with --no-verify to make diffs almost inverse of each other
.layout__2-sidebars for when the right sidebar is to be hidden on mobile .layout__2-sidebars-inline for when the right sidebar is to be put between content header and body on mobile
otherwise we can't consistently position the overlayed sidebar: scroll to top of page, resize page width to 790px and toggle sidebar to repro keeping both media queries because --screen-layout-no-sidebar may change to be smaller than --screen-small-and-narrower in the future again (we don't want to have to go update other files as we change our min content and sidebar sizes and paddings)
urls for testing each renderer: CurriculumAbout: http://localhost:3000/en-US/curriculum/about-curriculum/ CurriculumDefault: http://localhost:3000/en-US/curriculum/faq/ CurriculumModule: http://localhost:3000/en-US/curriculum/getting-started/soft-skills/ CurriculumOverview: http://localhost:3000/en-US/curriculum/getting-started/
Contributor
|
8d357e9 was deployed to: https://fred-pr1154.review.mdn.allizom.net/ |
Member
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initially a fix for #690 (comment), increasing the gap between content and the sidebar content by 1rem, but I ended up noticing more bugs and wanting to fix them wholesale with a unified approach to how we do our 2 sidebar layout across docs, curriculum and generic content.
Commits are atomic, and some have lengthy explanations in their commit message: I'd recommend reviewing one by one.
Fixes these issues:
If we like this approach, we should probably migrate all the other layouts using the variables to a class-based system, but it felt like I'd got to a coherent and consistent place with this so stopped here.