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

ToC and Chapter list iteration 1 #453

Closed
adamwoodnz opened this issue Dec 21, 2023 · 6 comments · Fixed by #469
Closed

ToC and Chapter list iteration 1 #453

adamwoodnz opened this issue Dec 21, 2023 · 6 comments · Fixed by #469
Assignees
Labels
Accessibility Issues related to keyboard navigation, screen readers, etc layout priority: high Redesign [Status] Needs Design [Type] Bug Something isn't working [Type] Enhancement New feature or request ui
Milestone

Comments

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Dec 21, 2023

There are multiple existing issues open for these components, so this issue aims to address all these together, settle on a new design and implement the changes. Summary of issues:

  1. ToC isn't sticky when it's longer than the viewport ToC is cumbersome, especially when ToC is long #445 TOC isn't position fixed when if it's too long #365
  2. Chapter list doesn't scroll with the page Chapters list: Make sticky, can potentially scroll independently #341
  3. Header hit area to expand on mobile is only the caret Chapter toggle zone should be larger on mobile (and desktop) #434

Technical considerations

Currently the Chapter list is a custom block in the Developer theme. The Table of Contents (and the Sidebar Container, which actually controls the sticky behaviour) are custom blocks in the wporg-mu-plugins repo. The Sidebar Container and Table of Contents blocks are also used on the Documentation single pages. To streamline development it would be worth consolidating the scrolling behaviour and header in a new or refactored mu-plugins block which is used for both.

@adamwoodnz adamwoodnz added [Type] Bug Something isn't working [Type] Enhancement New feature or request layout ui Accessibility Issues related to keyboard navigation, screen readers, etc priority: high labels Dec 21, 2023
@adamwoodnz adamwoodnz added this to the Iteration 1 milestone Dec 21, 2023
@adamwoodnz adamwoodnz self-assigned this Dec 21, 2023
@adamwoodnz
Copy link
Contributor Author

adamwoodnz commented Dec 21, 2023

@WordPress/meta-design there have been discussions about having these components fixed, with height set to fit between the header and footer and independent internal scroll. Is this still the preferred design?

Collapsible behaviour for the chapter list has also been discussed but I think can be addressed separately as an extra wrapper.

cc @ndiego

@jasmussen
Copy link
Collaborator

Here's a basic codepen for how I think it could/should work: https://codepen.io/joen/pen/JjzPQMR?editors=1100

  • Toc and chapters are both sticky or fixed
  • Both have a max-height of 100% and overflow if needed
  • There's custom CSS for the scrollbars that work for both Safari/Chrome and the standards based Firefox rules
  • The scrollbars appear on hover, or always on mobile.

@adamwoodnz
Copy link
Contributor Author

adamwoodnz commented Dec 22, 2023

Thanks, that's what I imagined.

  • The scrollbars appear on hover, or always on mobile.

Are we keeping the inline collapsed behaviour on mobile? In that case I don't think they'd scroll.

The other thing we have to consider with these layouts is how these scrollable areas work with the sticky header and full width footer. I imagine they will scroll up with the content until they hit the space below the header, then become fixed (like the current ToC). As the footer plus any full width content above scrolls up into the view, the height of the scrollable areas will need to reduce, or the whole thing will need to start scrolling up (as it does currently). This area below the content can be very tall, see what I mean by scrolling to the bottom of a page like https://developer.wordpress.org/reference/classes/wp_query/ when logged in:

Reducing the height of the lower content by pulling the feedback notes into the content column could help. In that case just reducing the height of the scrollable area could work, and would probably be simpler to implement.

@jasmussen
Copy link
Collaborator

Are we keeping the inline collapsed behaviour on mobile? In that case I don't think they'd scroll.

Yes, sorry I shouldn't have added that comment on always scrolling on mobile. As I wrote the comment I was mainly thinking of this code from the pen, and wanting to explain it:

@media (hover: none) {
	.scrollable {
	  visibility: visible;
	}
}

Essentially the visibility trick is what shows/hides the scrollbar depending on hover state. In that moment I forgot we collapse the two to pill shapes on mobile, of course we keep that. As you were.

The other thing we have to consider with these layouts is how these scrollable areas work with the sticky header and full width footer. I imagine they will scroll up with the content until they hit the space below the header, then become fixed (like the current ToC). As the footer plus any full width content above scrolls up into the view, the height of the scrollable areas will need to reduce, or the whole thing will need to start scrolling up (as it does currently). This area below the content can be very tall, see what I mean by scrolling to the bottom of a page like https://developer.wordpress.org/reference/classes/wp_query/ when logged in:

Yes, I recognize this is tricky. Impossible?

Reducing the height of the lower content by pulling the feedback notes into the content column could help. In that case just reducing the height of the scrollable area could work, and would probably be simpler to implement.

Can you elaborate a bit on this to help me understand? One solution just extrapolating from the screenshot (yes I forgot those notes), is that I can provide you with a design that puts them below the comment box, keeps them in the center column. Would that help?

@adamwoodnz
Copy link
Contributor Author

adamwoodnz commented Dec 28, 2023

Impossible?

🙂 No, we already have code running to detect the bottom collision, we'll just need to modify the behaviour.

Can you elaborate a bit on this to help me understand? One solution just extrapolating from the screenshot (yes I forgot those notes), is that I can provide you with a design that puts them below the comment box, keeps them in the center column. Would that help?

Yeah I think it would help, something like this is what I meant. Because only the footer will collide, the ToC can just reduce in height, rather than having to begin scrolling up.

developer wordpress org_reference_classes_wp_query_(Desktop)

@jasmussen
Copy link
Collaborator

That sounds good ot me. I might fiddle with the space above the notes, or even add sort of a subheading, but otherwise I think that sketch you have can work.

adamwoodnz added a commit to WordPress/wporg-mu-plugins that referenced this issue Jan 31, 2024
Causes space between content and sidebar to be reduced on Documentation site.

See WordPress/wporg-developer#453
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Issues related to keyboard navigation, screen readers, etc layout priority: high Redesign [Status] Needs Design [Type] Bug Something isn't working [Type] Enhancement New feature or request ui
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants