MarkBind / markbind

MarkBind is a tool for generating content-heavy websites from source files in Markdown format
https://markbind.org/
MIT License
134 stars 123 forks source link

Allocate space for scrollbar in sitenav and pagenav #2491

Closed jingting1412 closed 3 months ago

jingting1412 commented 3 months ago

What is the purpose of this pull request?

Overview of changes:

2217

Changed the property value of overflow-y of the .nav-component to scroll to allocate space to the scrollbar as discussed here so that its appearance and disappearance doesn't cause the icon in the nav bar to shift.

Since this change is to .nav-component, both the sitenav and pagenav is affected.

Since the change is to the main.css file of the default template, all the other main.css files also have to be updated. Currently there isn't a way to automatically update the file after a MarkBind upgrade (#1961), so if this change is accepted, there has to be release note for current users to update their main.css file.

Anything you'd like to highlight/discuss: overflow-y: overlay is a legacy value alias for overflow-y: auto according to the mdn docs here, which unfortunately still causes the moving of the content from what I can see.

The overlay value of the overflow CSS property overall is being deprecated and standardised as the scrollbar-gutter property, but I see that its not really supported on Safari browser. If anyone wants me to look into using this property instead, or any other alternatives, I'll be happy to try it out.

Previous behaviour:

https://github.com/MarkBind/markbind/assets/105090139/3e97f9fa-3d0c-4bb6-adb8-6450bcabe48f

Current behaviour:

https://github.com/MarkBind/markbind/assets/105090139/2e83f9a2-2440-4136-97e4-be21ef0cf0c7

Testing instructions: Serve the docs using markbind serve -d, collapse all sections of the siteNav, open the sections one by one until the scrollbar appears. Ensure that when the scrollbar appears, the icons doesnt shift.

Proposed commit message: (wrap lines at 72 characters) Allocate space for scrollbar in nav components


Checklist: :ballot_box_with_check:


Reviewer checklist:

Indicate the SEMVER impact of the PR:

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).
codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 51.14%. Comparing base (69ec838) to head (625134d).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2491 +/- ## ======================================= Coverage 51.14% 51.14% ======================================= Files 124 124 Lines 5359 5359 Branches 1152 1152 ======================================= Hits 2741 2741 Misses 2328 2328 Partials 290 290 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

LamJiuFong commented 3 months ago

Thanks for the work @jingting1412!

I took a look at scrollbar-gutter and if I'm not wrong scrollbar-gutter does the same thing as overflow-y: scroll right?

I am good with having a blank space here (next to the sitenav) while hovering

Screenshot 2024-04-03 at 1 33 41 AM

not sure how others think

yucheng11122017 commented 3 months ago

Hmm does this mean that the scrollbar will be present even if the site-nav components' height is small and doesn't need a scrollbar?

jingting1412 commented 3 months ago

Hmm does this mean that the scrollbar will be present even if the site-nav components' height is small and doesn't need a scrollbar?

The space allocated for the scrollbar will always be there yes but the scrollbar will only appear when it overflows. The behaviour is the same as

overflow: overlay
scrollbar-gutter: stable
yucheng11122017 commented 3 months ago

Hmm does this mean that the scrollbar will be present even if the site-nav components' height is small and doesn't need a scrollbar?

The space allocated for the scrollbar will always be there yes but the scrollbar will only appear when it overflows. The behaviour is the same as

overflow: overlay
scrollbar-gutter: stable

I see. Thanks for the explanation!