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.

Sign up for GitHub

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

Jump to bottom

refactor(theme): improve robustness and readability of outline component #3368

Merged
merged 4 commits into from
Jan 3, 2024
Merged

refactor(theme): improve robustness and readability of outline component #3368

merged 4 commits into from
Jan 3, 2024

Conversation

zhangyx1998
Copy link
Contributor

@zhangyx1998 zhangyx1998 commented Dec 21, 2023 •
edited
Loading

fixes #3378

1. Enhance Data Consistency

The original code uses different data paths for generating and retrieving "anchors":

  1. Upon page content update, getHeaders() will be invoked.
    It will use querySelectorAll('.VPDoc :where(h1,h2,h3,h4,h5,h6)') to locate all header elements and then filter them using resolveHeaders().

  2. The processed list is then passed to VPDocAsideOutline.vue for rendering the outline.

  3. However, when handling scroll event, setActiveLink() uses two querySelectorAll statements to retrieve the list of anchor elements. It is assuming every header MUST have an associated anchor element inside or next to it, which is not always true!

    For example:

    Many issues have mentioned user-generated content which will not go through markdown transformers.
    Further away, in my use case, I used "v-html" directive to avoid expensive template parsing, which works fine with the outline rendering, but breaks the ActiveLink feature due to lack of anchor element or inappropriate classNames (i.e. .header-anchor)

The code provided by this PR fixes this issue by reusing the same data used for rendering the outline.

  1. A global variable resolvedHeaders is provided and will be updated upon each call to resolveHeaders(). This variable is an array keeping track of all header elements and their assigned hash links that will be used in the outline.

  2. When handling scroll events, instead of invoking querySelectors, setActiveLink() will directly use the global variable resolvedHeaders for the links currently displayed in the outline.

  3. In this way, activeLink handlers will always be strictly aligned with the displayed outline. And will cover a much wider senario.

2. Improve Robustness

The original getAnchorTop() function could fail at some special conditions. For example, when header element is nested in a positioned element:

<body>
  <!--other content-->
  <div position="relative">
    <div>
      <h2 id="example">My parentElement's offsetTop is zero!</h2>
    </div>
  </div>
</body>

There exist other conditions when a node should not be used for highlighting. I've covered them in detail in the comments of the commited code.

An alternative function getAbsoluteTop() in this PR will replace getAnchorTop(). The alternative solution is supposed to be more robust. It recursively accumulates relative offset to get a more reliable result. In addition, it returns NaN for unexpected items, so the handler may safely skip them.

3. Improve Code Readability

Function isAnchorActive() is only used by setActiveLink() to calculate the appropriate link to highlight. However, it was stripped out of context and the meanings of its return values [boolean, string | null] are not clear, especially for first-time readers.

In addition, the logic of checking isBottom is located inside setActiveLink(), but the logic to check if the page is on top is somehow nested inside isAnchorActive(), making it more difficult to understand.

In the pr code, I moved the logic back into isAnchorActive() and added comments to make things more clear.

@zhangyx1998 zhangyx1998 mentioned this pull request Dec 26, 2023
Closed
4 tasks
@brc-dd brc-dd self-assigned this Dec 27, 2023
@brc-dd brc-dd merged commit 3654470 into vuejs:main Jan 3, 2024
7 checks passed
@zhangyx1998 zhangyx1998 deleted the refactor/improve-outline-logic branch January 3, 2024 10:51
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@brc-dd brc-dd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[default theme] activeAnchor inconsistent with outline when embedding html elements
2 participants
@zhangyx1998 @brc-dd