Skip to content

[sticky otp/toc] Builds on elastic/docs #2478 #1

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

Merged
merged 5 commits into from
Jul 21, 2022

Conversation

colleenmcginnis
Copy link

Targets elastic#2478

I couldn't help but spend some time tinkering! So far this PR includes:

  • Refining the IntersectionObserver to highlight only one item at a time in the "On this page" list. (Current logic highlights the first heading that is visible on the page if there are multiple headings visible.)
  • Widens the container using a new content-container class to better accommodate sticky content on both sides.
  • Styles the table of contents to be more accessible with higher contrast between the text and the background (the blue on gray was difficult to read).
  • Uses SVGs for the "+" icons for the table of contents so they look crisp on screens with any resolution.
  • Simplifies the styling of the "Most popular" box.
clipped-shorter.mov

If you think this is a good direction (and not to controversial!) we can merge this into elastic#2478 and continue from there. There are a few things that definitely need to be taken care of still including:

  • A full review of the layout at various screen size breaks and browser types.
  • Applying the styling from the "Most popular" box to the generated "Recommended for you" box.
  • Injecting the "{Book title}" into the table of contents.
  • Testing on more pages outside the Observability guide.
  • Style the landing page.
  • I'm sure we'll find more!!

Let me know what you think about this direction!

Comment on lines 88 to 89
<!-- Can we pull this in? -->
<div class="book-title">{Book name}</div>
Copy link
Owner

Choose a reason for hiding this comment

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

If we can, it wouldn't be done here. We'd need to prepend the book name to the TOC when it's built.

Copy link
Owner

Choose a reason for hiding this comment

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

I'll look at how we access the book title on index pages. Maybe we can use the same logic.

Copy link
Owner

@bmorelli25 bmorelli25 Jul 20, 2022

Choose a reason for hiding this comment

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

On second thought, I think this is a non-issue. The title of the book and version selector are omitted when building locally. When doing a full build, the title and version appear at the top of the TOC:

Screen Shot 2022-07-20 at 1 23 55 PM

The title uses the id book_title.

Copy link
Author

Choose a reason for hiding this comment

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

First pass looks like:

Screen Shot 2022-07-21 at 2 24 35 PM

Copy link
Owner

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

This looks great! I'm going to merge into the main PR so we can get a PR build up!

@bmorelli25 bmorelli25 merged commit 3a73e85 into bmorelli25:sticky-otp Jul 21, 2022
@colleenmcginnis colleenmcginnis deleted the sticky-otp-cmcg branch July 21, 2022 20:24
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.

2 participants