Add scrolling to Organized Content sidebar on smaller desktop screens #322
Merged
a258wang
merged 5 commits from amy-organized-content-sidebar-scroll
into main
1 year ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'amy-organized-content-sidebar-scroll'
Deleting a branch is permanent. It CANNOT be undone. Continue?
Closes #314
Personally I think it feels a little finicky, but please let me know what y'all think!
position: sticky;
height: 100%;
margin: calc(8rem / 16) calc(32rem / 16) 0 0;
height: calc(94vh - (20rem / 16));
?
100vh
meant that it would still be a little bit cut off, so that's where the94vh
comes from.height: 94vh
, I noticed that the sidebar would still get a little bit clipped when the screen was small, so I added the- (20rem / 16)
to mitigate that. I chose20 rem / 16
to match the bottom margin of the sidebar.Looking back at this, I wonder if changing the margins to padding and settingEDIT: Since padding appears to be included in determining the scrollable, this doesn't really work.box-sizing: border-box
or something would allow us to simplify theheight
?This works for me!
What do you mean by "finicky"?
I'm glad to hear that it works! I'm just not sure if sidebar looks good on all different sizes of screens, plus the code is a bit messy 😅
This works, but I think we should create a div inside the sticky div, and make that overflow-able. That way we get to keep things clean.
Hmmm, it seems like
position: sticky
andoverflow: auto
need to be set on the same element in order for everything to work properly. I've pushed a new commit where I put these attributes on a surrounding div instead of directly on the nav element - please let me know what y'all think!(Also I realized that in my haste, I completely messed up the mobile styling, so regardless of whether or not we want to keep the changes from commit
6e141dc2c2
, I'll probably have to make more changes, oops.)Add scrolling to Organized Content sidebar on smaller desktop screensto WIP: Add scrolling to Organized Content sidebar on smaller desktop screens 1 year ago.nav {
.navWrapper {
top: calc(20rem / 16);
bottom: calc(60rem / 16);
I don't think this is doing anything. Is it?
okay i think there's a very marginal improvement in terms of code quality between the new version and the old. So I'll leave it to you to decide what you wanna choose.
WIP: Add scrolling to Organized Content sidebar on smaller desktop screensto Add scrolling to Organized Content sidebar on smaller desktop screens 1 year agob36210f77a
to6df2f3fdd9
1 year ago92bf4752cd
into main 1 year agoReviewers
92bf4752cd
.