Move the sidebar to be stuck to left of the content #999

EmileTrotignon · 2023-09-15T10:00:45Z

I propose to change the position of the sidebar that contains the table of content to be closer to the content.
It used to be like that :

Here, the sidebar is quite far from the content. With a smaller screen this may be better.

My proposal is the following :

I think it is better for multiple reasons. First of all, you need to move your eyes and mouse less to use the table of content. Also, the previous design did not make it clear that the sidebar contained the table of content for the current page. Because of its position, it felt like a general sidebar for the whole website, and not linked to the current content. My proposal alleviate that issue by placing putting the sidebar closer to the content and under the title of the page.

It would also allow for a more consensual design for the search bar in #972

Julow

I like the new style very much! The previous side bar was invisible. This feels more conventional.

There was unwanted margin when the window is half-width that was probably here to accommodate the previous side bar. I have an improved margin/padding in this branch: https://github.com/Julow/odoc/tree/999-review
Feel free to cherry-pick.

Julow · 2023-09-20T14:39:00Z

src/html_support_files/odoc.css

+  height: fit-content;
+  border: solid 1px var(--border);
+  border-radius: 5px;
+  position:sticky;


Sticky is interesting but it has a behavior I find confusing:
If the side bar is longer than the screen is high but is smaller than the content, it's not possible to see the bottom of the side bar until we reach the bottom of the page.

Is here a way to make the side bar sticky only if the bottom of it is visible ?
Otherwise, I think I slightly prefer having a unsticky side bar. It's much more intuitive.

Note: We might make the side bar longer in the future.

Is here a way to make the side bar sticky only if the bottom of it is visible ?

I agree that would be nice. It is certainly possible with javascript. I will look for a css only solution.
I think the sticky is important because the bar is use for navigation inside the page itself.

Another solution is to have a maximal height for the left bar of the size of height, and add a scroll bar.

However, I do not love any of the solution!

Sticky, bottom of long nav bar inaccessible unless at the bottom of the page is a "no" for me

"Custom stickiness", with some js solving the above problem: why not, but not very satisfying to have to use JS.

Not sticky: simple and works, but not the best since we quickly lose the table of content in long pages

I'll be ok with those last two solutions.

There's no perfect solution and I think the scrollable side bar is the best compromise of usability and simplicity.

Julow · 2023-09-20T14:43:40Z

src/html_support_files/odoc.css

-  max-width: 100ex;
+  max-width: 122ex;
+  display: grid;
+  grid-template-columns: 28ex 90ex;


What is the goal of this? Shouldn't the other property already control the width ?

This cause overflow when the window is half the width of my screen.

This is how the grid layout usually work, but there this the fr unit I should use here for small width cases

The grid elements definitely have a size already. Removing this property removes the overflow in my testing.

Yes you are in fact right, the content does not have a width, but the sidebar does, and the body has a max witdh.

gpetiot · 2023-09-26T02:32:25Z

I notified the ocamlorg team about this issue in case they got some insight about the css

src/html_support_files/odoc.css

panglesd · 2023-09-28T15:33:32Z

src/html_support_files/odoc.css

+.odoc-content > *:first-child {
+  /* This make the first thing in the content align with the sidebar */
+  padding-top: 0;
+  margin-top: 0;
+}


The header (the first child) is not always small, and that makes strange result, I think.
See for instance the "odoc for authors" page of odoc's doc:

Why not, but I fear a much longer header would make it look strange, if the toc is pushed too much below...

I am not sure I see what you mean

Ah ! indeed. This is unrelated to that particular rule though. This can be changed by tweaking the grid layout (grid-row and grid-columns rules) (and if we do this rule becomes useless)
I agree it is a bit weird like that, but I think I still like it. I will ponder that a bit more.

panglesd · 2023-09-28T15:37:24Z

src/html_support_files/odoc.css

+  height: fit-content;
+  border: solid 1px var(--border);
+  border-radius: 5px;
+  position:sticky;


Another solution is to have a maximal height for the left bar of the size of height, and add a scroll bar.

However, I do not love any of the solution!

Sticky, bottom of long nav bar inaccessible unless at the bottom of the page is a "no" for me

"Custom stickiness", with some js solving the above problem: why not, but not very satisfying to have to use JS.

Not sticky: simple and works, but not the best since we quickly lose the table of content in long pages

I'll be ok with those last two solutions.

sabine · 2023-10-02T08:27:55Z

Ok so I'm back and I checked this out.

IMO sticky sidebar as a concept here is an okay solution since you don't have the problem OCaml.org had with a footer spanning all columns of the design (which would push the sticky sidebar out at the top when you go into the footer).

But here's what you need to do to make it work:

need to use max-height:100vh to make sure we can scroll the sidebar if it's too large
when using positioning like top: 50px (please make it smaller than 50px), you would need to use max-height: calc(100vh - 50px) instead. Also, now you can scroll only within the actual content box (the one that has the background). So what you'd probably want to do instead is to have a transparent wrapping container for the max-height: 100vh which is sticky and scrollable. Then, have the content box with background sit inside that container with some margin.
You can play around with bottom margin or padding of the surrounding container to make sure that the scrolling goes as far as it feels "right". Usually, you would have to have enough bottom padding to make up for the height of the page "header", i.e. the space between the very top of the page and the location where the sticky container sits before you scroll - otherwise, people cannot scroll the bottom of the content of the sidebar into view.
However, in this case, it's irregular because the preamble sits above the sidebar... maybe a third to a half of the page height would do as bottom padding. On pages with little content, the only consequence is that there's more white space at the bottom - which seems tolerable.

In general, it's simpler to work with fixed position layouts.

gpetiot · 2023-10-12T02:11:17Z

On _build/default/_doc/_html/odoc/odoc_for_authors.html the menu is not scrollable and the bottom of it might indeed not be displayed, I'm not sure if that is what Paul-Elliot was also reporting.
The whole css looks a bit overcomplicated to me, could we do something simpler like: https://www.w3schools.com/css/tryit.asp?filename=trycss_navbar_vertical_fixed that would mean having the preamble on the same column as the body.

EmileTrotignon · 2023-10-17T13:44:27Z

@gpetiot I do not think the css you linked to me would be able to achieve the result I desire, at least not without changing the html. I think Sabine's suggestion would fix the issue you mentionned.

EmileTrotignon · 2023-10-17T14:26:33Z

I have pushed commits that change the proposal into this :

It also has a scrollable table of content when its too big.

Julow

I love the new side bar as it is now :) I'm in favor of merging.

panglesd · 2023-10-31T12:32:19Z

This needs rebasing. I tried a "naive" resolution of conflicts, but it makes the search bar at the bottom of the page...

panglesd · 2023-10-31T12:34:57Z

Also, the changes introduced in the html layout in the search PR changes the way it looks compared to your last screenshot:

EmileTrotignon · 2023-11-07T14:36:35Z

I have made a new search compatible-layout. It looks like the following :

Julow · 2023-11-07T14:52:41Z

src/html_support_files/odoc.css

-  --toc-top: 20px;
-
+  /* TODO : make this smaller if there is no search bar */
+  --toc-top: calc(var(--search-bar-height) + var(--search-padding-top) + 20px);


Is the search bar sticky as well ? If not, this is not needed at all, --toc-top: 20px is sufficient.

Unfortunately the search bar is sticky and I find that not satisfying. The search bar takes valuable vertical space and is not properly spaced from the content is hovers. It's only surrounded by a white shadow that makes it hard to spot where the contents begins and doesn't fit with Odoc's theme.

When the search bar is not present, this calculation adds space above the TOC that doesn't look good either.

I suggest making the search bar non sticky to fix all of that.

is not properly spaced from the content is hovers

i do not understand this part

When the search bar is not present, this calculation adds space above the TOC that doesn't look good
either.

I can fix this.

It's only surrounded by a white shadow that makes it hard to spot where the contents begins

This is probably fixable, although I do not remember having this issue

doesn't fit with Odoc's theme.

Its true the input element is practically unstyled, but I plan to style it in my colors pr. As of now, it is dependent on browser defaults, which are very different and sometimes ugly.

I like the sticky sidebar, for the reason that I think that searching for something while looking at the doc is very useful. If it is not sticky, if you read the doc and the want to search for something, then you have to scroll to the top, and might forget what you wanted to do, and you also cannot type the query while looking at the doc (for instance copying a typename in a type-search query).
Also, the sidebar was not made sticky by this PR, and changing it is a little out of scope.

is not properly spaced from the content is hovers

I mean that it's hard to distinguish where the content starts below the search bar.

If it is not sticky, if you read the doc and the want to search for something, then you have to scroll to the top, and might forget what you wanted to do, and you also cannot type the query while looking at the doc

I don't think this workflow is well implemented currently. The dropdown showing the search results is above the page and might interfere. On mobile, almost none of the page is visible while typing the search query.
This waste vertical space at all time, not just when I intent to search.

Also, the sidebar was not made sticky by this PR, and changing it is a little out of scope.

Agree! I just noticed that the search bar was sticky while reviewing this calculation. Let's not do change the search bar in this PR.

On mobile, almost none of the page is visible while typing the search query.

True, the search bar should not be sticky on mobile

This waste vertical space at all time, not just when I intent to search.

That is true, there is a tradeoff here. Maybe with some js magic we can have both, but I am not sure.

I would trade having to scroll to the top for more vertical space.

test/search/html_search.t/run.t

Julow · 2023-11-07T15:06:04Z

This needs an other rebase. I think the commits could be squashed as the history is currently a bit messy.

EmileTrotignon · 2023-11-08T12:06:50Z

Is the search bar sticky as well ? If not, this is not needed at all, --toc-top: 20px is sufficient.

Unfortunately, this does not work.

When the search bar is not present, this calculation adds space above the TOC that doesn't look good either.

I have pushed a fix for this.

EmileTrotignon · 2023-11-13T14:22:23Z

I have squashed everything, I believe this is now ready for merging

Julow

This needs a rebase and then can be merged.

I'd like a comment in the code if you have the time.

Julow · 2023-12-05T14:12:56Z

src/html_support_files/odoc.css

@@ -759,19 +783,29 @@ td.def-doc *:first-child {
  line-height: 1.2;
 }

+.odoc-search + * + .odoc-toc {


A comment saying that this is to accommodate the sidebar would be useful.

this is done

Julow · 2023-12-05T16:31:54Z

Perfect, let's merge!

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

EmileTrotignon requested a review from panglesd September 15, 2023 13:20

EmileTrotignon force-pushed the sidebar-middle branch from 2166c29 to 54b200c Compare September 19, 2023 12:10

Julow approved these changes Sep 20, 2023

View reviewed changes

panglesd reviewed Sep 28, 2023

View reviewed changes

EmileTrotignon force-pushed the sidebar-middle branch from 24e73e8 to 1c120e5 Compare October 17, 2023 14:16

Julow approved these changes Oct 30, 2023

View reviewed changes

EmileTrotignon force-pushed the sidebar-middle branch from 1c120e5 to ef2ea1c Compare November 7, 2023 14:36

Julow reviewed Nov 7, 2023

View reviewed changes

EmileTrotignon force-pushed the sidebar-middle branch from dd9a39e to 2585477 Compare November 13, 2023 13:47

EmileTrotignon force-pushed the sidebar-middle branch 2 times, most recently from 38ed355 to b153813 Compare November 24, 2023 14:00

EmileTrotignon added this to the 2.4.0 milestone Dec 5, 2023

Julow approved these changes Dec 5, 2023

View reviewed changes

EmileTrotignon added 2 commits December 5, 2023 16:34

Move the sidebar to be stuck to left of the content

51a0f13

changes

51baa29

EmileTrotignon force-pushed the sidebar-middle branch from b153813 to 51baa29 Compare December 5, 2023 15:34

EmileTrotignon added 2 commits December 5, 2023 16:37

add comment

9ac5b54

support files

c027a90

Julow merged commit 99bcd17 into ocaml:master Dec 5, 2023
5 checks passed

Julow mentioned this pull request Dec 11, 2023

[new release] odoc and odoc-parser (2.4.0) ocaml/opam-repository#24927

Closed

Julow mentioned this pull request Dec 12, 2023

[new release] odoc and odoc-parser (2.4.0) ocaml/opam-repository#24930

Merged

polytypic mentioned this pull request Apr 16, 2024

Mind the gap #1111

Open

This was referenced Jun 10, 2024

Less gaps in css #1143

Closed

Fix gaps between preamble and content #1146

Closed

Remove margin of first child of odoc-content #1147

Merged

Move the sidebar to be stuck to left of the content #999

Move the sidebar to be stuck to left of the content #999

Conversation

EmileTrotignon commented Sep 15, 2023 • edited Loading

Julow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panglesd Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gpetiot commented Sep 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panglesd Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

sabine commented Oct 2, 2023

gpetiot commented Oct 12, 2023

EmileTrotignon commented Oct 17, 2023

EmileTrotignon commented Oct 17, 2023

Julow left a comment

Choose a reason for hiding this comment

panglesd commented Oct 31, 2023

panglesd commented Oct 31, 2023

EmileTrotignon commented Nov 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmileTrotignon Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmileTrotignon Nov 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Julow commented Nov 7, 2023

EmileTrotignon commented Nov 8, 2023 • edited Loading

EmileTrotignon commented Nov 13, 2023

Julow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Julow commented Dec 5, 2023

EmileTrotignon commented Sep 15, 2023 •

edited

Loading

panglesd Sep 28, 2023 •

edited

Loading

panglesd Sep 28, 2023 •

edited

Loading

EmileTrotignon Nov 7, 2023 •

edited

Loading

EmileTrotignon Nov 8, 2023 •

edited

Loading

EmileTrotignon commented Nov 8, 2023 •

edited

Loading