feat(v2): highlight items in the table of content #1896

SantiagoGdaR · 2019-10-27T04:00:08Z

Motivation

Implement the highlight functionality for the table of content links on the scroll and on click events.
The implementation is partially based on the solution implemented in v1 but it is drastically changed to make better use of React.

Resolve issue: #1752
Also relates to: #1524

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

When clicking on any of the links of a table of content or while you scroll on a section that contains a table of content you are going to notice that the table of content has the link of the section you have on the screen highlighted.

Tested on Safari and Chrome.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

docusaurus-bot · 2019-10-27T04:05:59Z

Deploy preview for docusaurus-preview ready!

Built with commit 542b7c9

https://deploy-preview-1896--docusaurus-preview.netlify.com

docusaurus-bot · 2019-10-27T04:06:21Z

Deploy preview for docusaurus-2 ready!

Built with commit 542b7c9

https://deploy-preview-1896--docusaurus-2.netlify.com

* Convert sitemap plugin to TypeScript Test - enabled the sitemap plugin in the v2 website and verified that the sitemap is created after running `docusaurus build`. * Addressing review comments

endiliey

If i go to https://deploy-preview-1896--docusaurus-2.netlify.com/docs/introduction/#gitbook

The Gitbook is not highlighted, i have to scroll around or resize first.

Compare it to https://docusaurus.io/docs/en/installation#running-the-example-website

Update: Clicking the right sidebar directly also doesnt highlight that particular section

lex111 · 2019-10-27T12:04:31Z

Strange it works fine for me in Chrome (I'm on Linux), but not in Firefox 🤷‍♂️

…cebook#1898) * perf(v2): reduce bundle size significantly with super short chunk name and registry * changelog * use hash:8 as id for better long term caching * even shorter filename. slice contenthash

* fix(v2): align search icon on small width device * nits * nits

wgao19 · 2019-10-27T16:14:59Z

There are many misbehaviors from preview:

does not pick up the correct color when navigating by url
clicking on level 2 items sometimes does not pick up the correct color
never highlights the first or the last item

The behaviors said above appears in both Firefox and Chrome, the screen record I have here shows Chrome:

I think the implementation in #1524 is correct and the comments around it serves as good pseudocode. Suggest that we follow it directly.

SantiagoGdaR · 2019-10-27T16:30:23Z

If i go to https://deploy-preview-1896--docusaurus-2.netlify.com/docs/introduction/#gitbook

The Gitbook is not highlighted, i have to scroll around or resize first.

Compare it to https://docusaurus.io/docs/en/installation#running-the-example-website

Update: Clicking the right sidebar directly also doesnt highlight that particular section

Both things work fine for me (the gitbook highlight works fine, also clicking the sidebar update directly) I am going to play around until I can reproduce it.

Strange it works fine for me in Chrome (I'm on Linux), but not in Firefox 🤷‍♂

OK, I am going to test in firefox and see what's going on.

There are many misbehaviors from preview:

does not pick up the correct color when navigating by url

clicking on level 2 items sometimes does not pick up the correct color

never highlights the first or the last item

The behaviors said above appears in both Firefox and Chrome, the screen record I have here shows Chrome:

I think the implementation in #1524 is correct and the comments around it serves as good pseudocode. Suggest that we follow it directly.

If you want me too I can see the issues that are happening (some of them also happen in v1).
About the comments, I am aligned with the Clean Code points of view, Comments are good if you need to explain something that can't be explained by writing descriptive code. In any other situation, you are probably just being redundant or your code is not well thought. But I can add comments around if that what you need.

Please tell me if you want me to fix the issues described in the comments or you prefer to delete this PR. I did put time on this but I completely understand if you prefer to go another way.
Thanks

endiliey · 2019-10-27T16:43:33Z

Both things work fine for me (the gitbook highlight works fine, also clicking the sidebar update directly) I am going to play around until I can reproduce it.

Here's the demo :)

endiliey

Actually my complain was just a nitpicker. Overall it works fine. Gonna leave it to @yangshun and @wgao19 for final approval

wgao19 · 2019-10-27T17:12:02Z

I think the issues need to be fixed before merging, the behavior doesn't feel correct to me atm.

endiliey · 2019-10-27T17:13:07Z

I wont merge until both of u approve :) Just putting stamp 😃

SantiagoGdaR · 2019-10-27T18:22:42Z

Great! I am going to work on the fixes. Thanks

* change(v2): refactor dark toggle into a theme * follow how themes resolve files * let theme hook to pick up default theme by itself

…acebook#1871) * fix issue#1752 when element in side nav is hovered over the color changes. * Update main.css

…1895)

yangshun

Nice work. This is one of the features that I'm highly anticipating too, thank you for working on this.

Tested on Chrome, Safari and Firefox, and contrary to what @lex111 experienced, the result on Firefox is the best 😮

Some suggestions I have:

Create a custom hook so that this is reusable by non-theme classic Docs components. In future if we want to have a Bootstrap theme, we can reuse this hook.
- The hook should take in the corresponding class names as function arguments.
- DocItem should import this hook and invoke it with the classnames specific to this theme.
You might want to take a look at the implementation of feat: highlight nav item in onPageNav ToC #1524 for inspiration on how this was achieved in v1. v1's behavior is great and battle-tested (no complaints about it so far). cc @tusharf5 if he's interested in helping to review.

packages/docusaurus-theme-classic/src/theme/DocItem/index.js

* misc(v2): address comments * misc(v2): update CHANGELOG

* feat(v2): allow line highlighting * Refactor: use parse-numeric-range for parsing * Add line highlighting for live code blocks * feat(v2): add sticky footer (facebook#1855) * feat(v2): add sticky footer * Update CHANGELOG-2.x.md * Update CHANGELOG-2.x.md * fix(v2): remove empty doc sidebar container (facebook#1870) * docs: showcase user Amphora (facebook#1873) * Add Amphora Data link to users Adds Amphora Data to the list of users * Add Amphora Data logo * fix case of image path * Move Image into users directory * use black amphora image * fix(v2): fix search input blur on desktop (facebook#1874) * docs(v2): showcase user mbt-bundle (facebook#1875) * feat(v2): postcss should only use stage 3 features instead of stage 2 (facebook#1872) * feat(v2): add ability expand all items in doc sidebar (facebook#1876) * feat(v2): add ability expand all items in doc sidebar * Fix tests * Refactor: use themeConfig * Improve docs * Revert unnecessary changes * Refactor: better consistency * Revert extra change * Refactor: use useDocusaurusContext to get config value * chore(v2): update changelog * chore(v2): update showcase and broken link * perf(v2): disable hash for css modules localident in dev (facebook#1882) * perf(v2): disable hash for css modules localident in dev * changelog * feat(v2): display footer in docs page for consistency (facebook#1883) * feat(v2): display footer in docs page * nits * address review * nits * docs(v2): fix format inline code (facebook#1888) * docs(v2): add docs on useful client api (facebook#1890) * docs(v2): add docs on useful client api * Update docusaurus-core.md * Update website/docs/docusaurus-core.md * Update website/docs/docusaurus-core.md * Update website/docs/docusaurus-core.md * Update website/docs/docusaurus-core.md * docs(v2): update config docs (facebook#1885) * fix(v2): do not show categories with empty items (facebook#1891) * styles(v2): update infima and fix styles (facebook#1892) * fix(v2): wrong css class * v2.0.0-alpha.31 * chore(v2): update docs and changelog * docs(v2): update plugins, presets and themes docs (facebook#1889) * docs(v2): update plugins, presets and themes docs * ideal image plugin * proof reading * Merge master * refactor(v2): Convert sitemap plugin to TypeScript (facebook#1894) * Convert sitemap plugin to TypeScript Test - enabled the sitemap plugin in the v2 website and verified that the sitemap is created after running `docusaurus build`. * Addressing review comments * perf(v2): significantly reduce bundle size & initial html payload (facebook#1898) * perf(v2): reduce bundle size significantly with super short chunk name and registry * changelog * use hash:8 as id for better long term caching * even shorter filename. slice contenthash * fix(v2): align search icon on small width device (facebook#1893) * fix(v2): align search icon on small width device * nits * nits * refactor(v2): refactor dark toggle into a hook (facebook#1899) * change(v2): refactor dark toggle into a theme * follow how themes resolve files * let theme hook to pick up default theme by itself * perf(v2): reduce memory usage consumption (facebook#1900) * misc(v1): use primary color for hovered items in table of contents (facebook#1871) * fix issue#1752 when element in side nav is hovered over the color changes. * Update main.css * fix(v1): mobile safari search input misalignment in header (facebook#1895) * misc(v2): v1 backward compatibility for USE_SSH env var (facebook#1880) * misc(v2): address comments * misc(v2): update CHANGELOG

* Update README.md - Fix Broken Link * misc: update URLs to non-HTML versions

…ok#1917) * fix(v2): regression from prioritizing core node_modules logic * nits

…facebook#1925)

lex111

@SantiagoGdaR don't do anything until you get approval from Docusaurus devs, this is just my 2 cents.

lex111 · 2019-11-02T13:12:00Z

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

+    let tableOfContentLinks = [];
+
+    function getActiveHeaderAnchor() {
+      const TOP_OFFSET = topOffset;


Do you really need to create a constant for this when it is used once? I mean, the code is not so big, and there is no point in duplicating the same value in such a way.

In my opinion it is better to do without it:

- if (top >= 0 && top <= TOP_OFFSET) {} + if (top> = 0 && top <= topOffset) {}

lex111 · 2019-11-02T13:15:42Z

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

+  ] = useState(undefined);
+
+  useEffect(() => {
+    let headersAnchor = [];


Is not it more correct to call this var that?

headersAnchor -> headerAnchors

lex111 · 2019-11-02T13:21:28Z

packages/docusaurus-theme-classic/src/theme/DocItem/index.js

@@ -11,16 +11,26 @@ import Head from '@docusaurus/Head';
 import useDocusaurusContext from '@docusaurus/useDocusaurusContext';
 import useBaseUrl from '@docusaurus/useBaseUrl';
 import DocPaginator from '@theme/DocPaginator';
+import useTableOfContentHighlight from '@theme/hooks/useTableOfContentHighlight';


Maybe it makes sense TableOfContent shorten to simply toc? It will be easier to read and understand.

useTableOfContentHighlight -> useTocHighlight

tableOfContentLinkClassName -> tocLinkClassName

And so on

lex111 · 2019-11-02T13:39:33Z

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

+      setLastActiveTableOfContentLink(tableOfContentLink);
+    }
+
+    function getAnchorValue(tableOfContentLink) {


I would consider this solution, it will be faster.

function getAnchorValue(tocLink) { return decodeURIComponent( tocLink.href.substring(tocLink.href.indexOf('#') + 1) ); }

lex111 · 2019-11-02T13:42:18Z

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

+    function getActiveHeaderAnchor() {
+      const TOP_OFFSET = topOffset;
+      let index = 0;
+      let activeHeaderAnchor;


You must probably explicitly set the default value.

let activeHeaderAnchor = null;

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

lex111 · 2019-11-02T14:00:12Z

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

+      if (activeHeaderAnchor) {
+        let index = 0;
+        let itemHighlighted = false;
+        tableOfContentLinks = document.querySelectorAll(


Hmm, I would prefer to use the old-school document.getElementsByClassName (sort of like it's even faster), since the selection goes only by class and it is assumed that only the class will be used, otherwise you need to rename the parameter to tableOfContentLinkSelector and continue use querySelectorAll 🤔

lex111 · 2019-11-02T14:05:09Z

packages/docusaurus-theme-classic/src/theme/hooks/useTableOfContentHighlight.js

+  const [
+    lastActiveTableOfContentLink,
+    setLastActiveTableOfContentLink,
+  ] = useState(undefined);


Use null instead of undefined

packages/docusaurus-theme-classic/src/theme/DocItem/index.js

SantiagoGdaR · 2019-11-02T20:25:45Z

Thanks @lex111, personally I agree on most of the changes you propose. I am going to work on them or respond with a comment in the case I think we should discuss something further.

* docs(v2): theme, plugin, and preset config * change tabs to spaces * change theme example

This is much stable and more performant

* fix(v2): allow to create tabs with only one item It was not possible to have tabs containing only one tab item, so the code below crashed ``` <Tabs defaultValue="SomeFile.js" values={[ { label: "SomeFile.js", value: "SomeFile.js" } ]} > <TabItem value="SomeFile.js"> Tab content </TabItem> </Tabs> ``` * Update index.js

yangshun

Looks good overall. Let's implement some of the suggestions by @lex111 and we're good to merge.

Thanks for working on this!

packages/docusaurus-theme-classic/src/theme/DocItem/index.js

lex111 · 2019-11-04T11:26:38Z

I understand correctly that we will implement all my suggestions, except the last one, about the concatenating of classes?

…into SantiagoGdaR-feat/1752

yangshun

I've made the changes. Merging it 🎉

Santiago Garcia da Rosa added 2 commits October 26, 2019 22:37

feat(v2): highlight items in the table of content

19efb7c

refactor: move string to const

4f44e36

facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 27, 2019

refactor(v2): Convert sitemap plugin to TypeScript (facebook#1894)

c23f981

* Convert sitemap plugin to TypeScript Test - enabled the sitemap plugin in the v2 website and verified that the sitemap is created after running `docusaurus build`. * Addressing review comments

endiliey requested review from yangshun and wgao19 October 27, 2019 09:11

endiliey reviewed Oct 27, 2019

View reviewed changes

endiliey added 2 commits October 27, 2019 07:09

perf(v2): significantly reduce bundle size & initial html payload (fa…

fabaf77

…cebook#1898) * perf(v2): reduce bundle size significantly with super short chunk name and registry * changelog * use hash:8 as id for better long term caching * even shorter filename. slice contenthash

fix(v2): align search icon on small width device (facebook#1893)

84462e2

* fix(v2): align search icon on small width device * nits * nits

endiliey approved these changes Oct 27, 2019

View reviewed changes

wgao19 and others added 4 commits October 27, 2019 13:02

refactor(v2): refactor dark toggle into a hook (facebook#1899)

01f4d9a

* change(v2): refactor dark toggle into a theme * follow how themes resolve files * let theme hook to pick up default theme by itself

perf(v2): reduce memory usage consumption (facebook#1900)

98679b5

misc(v1): use primary color for hovered items in table of contents (f…

f14b6ee

…acebook#1871) * fix issue#1752 when element in side nav is hovered over the color changes. * Update main.css

fix(v1): mobile safari search input misalignment in header (facebook#…

421598e

…1895)

yangshun mentioned this pull request Oct 27, 2019

[v2] use primary color for highlighted items in table of contents #1752

Closed

yangshun suggested changes Oct 27, 2019

View reviewed changes

yns88 and others added 5 commits October 27, 2019 14:36

misc(v2): v1 backward compatibility for USE_SSH env var (facebook#1880)

812a30b

* misc(v2): address comments * misc(v2): update CHANGELOG

docs(v1): remove user Vasern (facebook#1901)

3172602

misc: update URLs to non-HTML versions (facebook#1902)

3f4c805

* Update README.md - Fix Broken Link * misc: update URLs to non-HTML versions

misc(v2): improve index page SEO score

b6667a0

endiliey and others added 8 commits October 30, 2019 13:02

fix(v2): regression from prioritizing core node_modules logic (facebo…

ca4efb6

…ok#1917) * fix(v2): regression from prioritizing core node_modules logic * nits

chore: changelog

c7ff07c

docs(v1): showcase user Reactive Interaction Gateway (facebook#1918)

255584c

chore: bump dependencies

ab15205

docs(v2): fix typo in advanced plugins (facebook#1926)

472c8fc

docs(v2): fix typos (facebook#1930)

b8335fe

docs(v1): fix links in CHANGELOG (facebook#1931)

0baaad3

fix(v2): @theme/heading should not create anchor if id is not defined (…

c41c19e

…facebook#1925)

endiliey requested a review from lex111 November 2, 2019 10:41

lex111 reviewed Nov 2, 2019

View reviewed changes

kabartolo and others added 8 commits November 3, 2019 17:59

docs(v2): theme, plugin, and preset config (facebook#1929)

3f8cb78

* docs(v2): theme, plugin, and preset config * change tabs to spaces * change theme example

misc: use /usr/bin/env bash to increase portability (facebook#1923)

82ece51

misc(v1): use Node.js lts version (facebook#1920)

33718b6

This is much stable and more performant

chore: downgrade imagemin (facebook#1933)

2d13fe2

fix(v2): add missing key prop in footer items with HTML (facebook#1935)

34a84a3

fix(v2): fix browser window background (facebook#1936)

146a301

fix(v2): remove redundant npm script in classic template (facebook#1937)

0d445fc

yangshun suggested changes Nov 4, 2019

View reviewed changes

packages/docusaurus-theme-classic/src/theme/DocItem/index.js Outdated Show resolved Hide resolved

endiliey added 2 commits November 4, 2019 12:45

v2.0.0-alpha.32

cd9e2f2

chore(v2): changelog

5909b49

wgao19 approved these changes Nov 4, 2019

View reviewed changes

yangshun added 3 commits November 5, 2019 09:01

Merge branch 'feat/1752' of https://github.com/SantiagoGdaR/docusaurus …

32773e5

…into SantiagoGdaR-feat/1752

misc(v2): update TOC highlight

77e8890

misc(v2): update CHANGELOG

542b7c9

yangshun approved these changes Nov 5, 2019

View reviewed changes

yangshun merged commit 3e58062 into facebook:master Nov 5, 2019

SantiagoGdaR deleted the feat/1752 branch November 5, 2019 03:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(v2): highlight items in the table of content #1896

feat(v2): highlight items in the table of content #1896

SantiagoGdaR commented Oct 27, 2019 •

edited

Loading

docusaurus-bot commented Oct 27, 2019 •

edited

Loading

docusaurus-bot commented Oct 27, 2019 •

edited

Loading

endiliey left a comment •

edited

Loading

lex111 commented Oct 27, 2019

wgao19 commented Oct 27, 2019

SantiagoGdaR commented Oct 27, 2019 •

edited

Loading

endiliey commented Oct 27, 2019

endiliey left a comment •

edited

Loading

wgao19 commented Oct 27, 2019

endiliey commented Oct 27, 2019

SantiagoGdaR commented Oct 27, 2019

yangshun left a comment

lex111 left a comment

lex111 Nov 2, 2019

lex111 Nov 2, 2019

lex111 Nov 2, 2019

lex111 Nov 2, 2019

lex111 Nov 2, 2019

lex111 Nov 2, 2019

lex111 Nov 2, 2019

SantiagoGdaR commented Nov 2, 2019

yangshun left a comment

lex111 commented Nov 4, 2019

yangshun left a comment

feat(v2): highlight items in the table of content #1896

feat(v2): highlight items in the table of content #1896

Conversation

SantiagoGdaR commented Oct 27, 2019 • edited Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

docusaurus-bot commented Oct 27, 2019 • edited Loading

docusaurus-bot commented Oct 27, 2019 • edited Loading

endiliey left a comment • edited Loading

Choose a reason for hiding this comment

lex111 commented Oct 27, 2019

wgao19 commented Oct 27, 2019

SantiagoGdaR commented Oct 27, 2019 • edited Loading

endiliey commented Oct 27, 2019

endiliey left a comment • edited Loading

Choose a reason for hiding this comment

wgao19 commented Oct 27, 2019

endiliey commented Oct 27, 2019

SantiagoGdaR commented Oct 27, 2019

yangshun left a comment

Choose a reason for hiding this comment

lex111 left a comment

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

lex111 Nov 2, 2019

Choose a reason for hiding this comment

SantiagoGdaR commented Nov 2, 2019

yangshun left a comment

Choose a reason for hiding this comment

lex111 commented Nov 4, 2019

yangshun left a comment

Choose a reason for hiding this comment

SantiagoGdaR commented Oct 27, 2019 •

edited

Loading

docusaurus-bot commented Oct 27, 2019 •

edited

Loading

docusaurus-bot commented Oct 27, 2019 •

edited

Loading

endiliey left a comment •

edited

Loading

SantiagoGdaR commented Oct 27, 2019 •

edited

Loading

endiliey left a comment •

edited

Loading