Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document section sidebar is always available #195

Merged
merged 6 commits into from
Oct 18, 2021

Conversation

yF-B
Copy link
Collaborator

@yF-B yF-B commented Sep 23, 2021

Document section sidebar now move with the page.

@yF-B
Copy link
Collaborator Author

yF-B commented Sep 27, 2021

If you want to test the changes you can copy the codes in your local folders. If you do not want to compile all the files you can use master branch to test it:

  1. clone the master branch of devonfw.github.io
  2. copy the codes in the files
  3. run your local server

Copy link
Member

@suprishi suprishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try to find a CSS-only solution, or something which relies less on JavaScript. You can try position: sticky on the sidebar.

Copy link
Member

@suprishi suprishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make all your changes within a media query targeting devices having width more than 768px so that it does not impact all other resolutions (mobiles and tablets).

@@ -352,6 +352,9 @@ b.button:after {
//margin-bottom: $panel-margin-bottom * 2;
border-bottom: $sect-divider-size $sect-divider-style $sect-divider-color;
padding-bottom: 0.5em;
overflow: -moz-scrollbars-none;
position: sticky !important;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

position: sticky is not making any difference here. Please apply it to #header. Also, position: sticky won't work on its own, you have to specify top: 0 so the browser knows where to "stick" it when you scroll.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously It worked for me locally so I don't know why it dose not work now. But in the new version I apply it to #header.

@@ -352,6 +352,9 @@ b.button:after {
//margin-bottom: $panel-margin-bottom * 2;
border-bottom: $sect-divider-size $sect-divider-style $sect-divider-color;
padding-bottom: 0.5em;
overflow: -moz-scrollbars-none;
position: sticky !important;
height: 570px !important;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than specifying a fixed height (570px), please use the calc() function so you can calculate the height based on the user's viewport, so that it looks good on all screen sizes.

You can calculate the height by adding up the height of the header and the footer and then subtracting the sum from 100vh (which means 100% of the viewport height).

So, for example, if the header's height is 40px and the footer's height is 340px and suppose we want to keep a 20px gap between the sidebar and the footer, then you can specify the height as height: calc(100vh - 400px).

You can find the heights by inspecting them in the developer tools of the browser.

@@ -380,6 +383,9 @@ b.button:after {
}
}
}
#toc::-webkit-scrollbar{
width: 0 !important
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please try to make the scrollbar visible on hovering over the sidebar?

@@ -6,6 +6,7 @@ body.book.toc2.toc-left {
}

#header {
width: 100% !important;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I want to get the height of the parent element of #header, but I think it is not necessary now.

Copy link
Member

@suprishi suprishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The height of the sidebar looks fine on larger screens but look very small in smaller laptops. But I don't see another alternative because if the height is taller then it overlaps the footer when it appears on the viewport.
The changes look good but there is more that can be done.

  1. Please put your changes within a media query targeting devices having width more than 1024px (@media (min-width: 1024px) { /* your changes */ }) so that it does not impact all other resolutions like mobiles and tablets like this:
    image

  2. As an enhancement, can you fade out the bottom edge of the sidebar. Right now, with the shortened height of the sidebar, its content is cut off abruptly and it looks ugly:
    image

  3. Also reduce the width of the sidebar (#toc) to around 90% so that there is some space between its scrollbar and the main content.

@suprishi suprishi merged commit 172e71d into devonfw:develop Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants