This repository was archived by the owner on Jan 8, 2025. It is now read-only.
Switch TabItems with identical labels together#380
Merged
Conversation
Fixes #317 If multiple `Tabs` components include `TabItem`s with identical labels, when a user switches to a particular `TabItem` within one of these `Tabs` components, switch the other `Tabs` components as well. This saves a user from clicking the same tabs repeatedly when navigating through the docs site. It also captures most of the usefulness of the scope switcher while being far more visible for users, who tend not to realize that the scope switcher exists. This will make it easier to eventually deprecate the scope switcher. This change adds a `TabContextProvider` high in the component tree (just below `DocsContextProvider`). `Tabs` components look up their currently selected tab from this context provider, rather than from component-local state. The `TabContextProvider` hashes the sorted label names of a `Tabs` component to get and set the value of the currently selected `TabItem`. `Tabs` components with a dropdown menu, as well as `Tabs` components in `dropDownView` mode, also use this context provider to find the current value of dropdown menu items. Note that the `DocsContextProvider`'s scope value currently supersedes the values managed by the `TabContextProvider`. For example, if a `Tabs` component on one page has the "Enterprise" dropdown menu item selected, and the user has the `oss` scope selected, a `Tabs` component with the same dropdown menu on another page will show "Open Source" instead of "Enterprise". We can change this by deprecating the docs scopes.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Contributor
Author
avatus
reviewed
Aug 21, 2023
Comment on lines
+21
to
+28
| const labelHashKey = (labels: Array<string>): string => { | ||
| const labelsCopy = [...labels]; | ||
| labelsCopy.sort(); | ||
| const hash = createHash("sha256"); | ||
| hash.update(labelsCopy.join("")); | ||
| return hash.digest("utf8" as BinaryToTextEncoding); | ||
| }; | ||
|
|
Contributor
There was a problem hiding this comment.
What is the reason for hashing? Why not just store as the label string
Contributor
Author
There was a problem hiding this comment.
I felt weird about having an unbounded key. I guess we'll never have hundreds of labels or anything, so this won't be a big deal. I've changed this to return the joined labels instead of hashing.
Comment on lines
+12
to
+20
| export const TabContext = createContext<TabContextProps>({ | ||
| getSelectedLabel: (tabs: Array<string>): string => { | ||
| return ""; | ||
| }, | ||
| setSelectedLabel: (tabs: Array<string>, selected: string) => { | ||
| return; | ||
| }, | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
I guess I haven't created a context with methods like this before. It looked so wrong to me but, the docs say this is how it goes so, 👍
Contributor
Author
There was a problem hiding this comment.
We use this approach in a few places in the docs engine. I'm not super familiar with React best practices, so I'm happy to change this (or create a GitHub issue so we can change this everywhere later) if there's a better way!
- Obtain a map key by joining tab labels, without hashing.
avatus
approved these changes
Aug 22, 2023
ptgott
added a commit
that referenced
this pull request
Aug 23, 2023
Fixes #343 Fixes #162 Fixes #155 Fixes #118 Fixes #116 Fixes #88 Users tend not to notice the scope switcher. When they do, the component's behavior is not intuitive. After we merged #380, tabs preserve their currently selected tab item as the user moves through the docs. This is the only aspect of the scope switcher that does not risk hiding unexpected information. Another aspect of the scope switcher, locking the version picker if the scope is currently `cloud` or `team`, causes unexpected results when Teleport Cloud is behind the latest self-hosted release. This change removes the scope switcher and replaces it with a list of available Teleport editions for a particular page, preserving the existing styling as much as possible. Aside from an "Available for" label next to the edition list, the change should appear minimal for users who do not interact with the scope switcher.
ptgott
added a commit
that referenced
this pull request
Aug 23, 2023
Closes #343 Closes #162 Closes #155 Closes #118 Closes #116 Closes #88 Users tend not to notice the scope switcher. When they do, the component's behavior is not intuitive. After we merged #380, tabs preserve their currently selected tab item as the user moves through the docs. This is the only aspect of the scope switcher that does not risk hiding unexpected information. Another aspect of the scope switcher, locking the version picker if the scope is currently `cloud` or `team`, causes unexpected results when Teleport Cloud is behind the latest self-hosted release. This change removes the scope switcher and replaces it with a list of available Teleport editions for a particular page, preserving the existing styling as much as possible. Aside from an "Available for" label next to the edition list, the change should appear minimal for users who do not interact with the scope switcher.
ptgott
added a commit
that referenced
this pull request
Aug 23, 2023
Closes #343 Closes #162 Closes #155 Closes #118 Closes #116 Closes #88 Users tend not to notice the scope switcher. When they do, the component's behavior is not intuitive. This change removes the scope switcher from the docs, replacing it with a non-interactive list of supported Teleport editions. The change preserves the existing styling of the scope switcher as much as possible. Aside from an "Available for" label next to the edition list, the change should go unnoticed for users who do not interact with the scope switcher. This change also adjust some docs behavior that relied on the scope switcher: - **The `DocsContext` no longer includes the current scope:** Components no longer refer to the `DocsContext` to determine the currently selected scope and adjust their visibility. "Box" components like the `Details`, `Notice`, and `Admonition` component can currently hide themselves if the selected scope does not match the value of the `scope` prop. This change shows these boxes at all times, but keeps the `scope` and `scopeOnly` props for backwards compatability (the props are no-op). The `Tabs` component currently uses the selected scope to select a `TabItem` with the `scope` property. Since we merged #380, though, `Tabs` components can preserve most of this behavior without the scope switcher, as long as these components include identical labels for edition-related tab items. This change also retains the `scope` prop in no-op form. - **Locking the version picker if the scope is `cloud` or `team`**: This change no longer locks the version picker, since there is no way to select a scope. Locking the version picker becomes an issue when Teleport Cloud is a major version behind the latest release. Removing the scope switcher doesn't completely rectify this, since the version of the docs for the previous version includes a warning banner. We can address this in a future change.
ptgott
added a commit
that referenced
this pull request
Sep 6, 2023
* Remove scoped visibility from the docs engine Closes #343 Closes #162 Closes #155 Closes #118 Closes #116 Closes #88 Users tend not to notice the scope switcher. When they do, the component's behavior is not intuitive. This change removes the scope switcher from the docs, replacing it with a non-interactive list of supported Teleport editions. The change preserves the existing styling of the scope switcher as much as possible. Aside from an "Available for" label next to the edition list, the change should go unnoticed for users who do not interact with the scope switcher. This change also adjust some docs behavior that relied on the scope switcher: - **The `DocsContext` no longer includes the current scope:** Components no longer refer to the `DocsContext` to determine the currently selected scope and adjust their visibility. "Box" components like the `Details`, `Notice`, and `Admonition` component can currently hide themselves if the selected scope does not match the value of the `scope` prop. This change shows these boxes at all times, but keeps the `scope` and `scopeOnly` props for backwards compatability (the props are no-op). The `Tabs` component currently uses the selected scope to select a `TabItem` with the `scope` property. Since we merged #380, though, `Tabs` components can preserve most of this behavior without the scope switcher, as long as these components include identical labels for edition-related tab items. This change also retains the `scope` prop in no-op form. - **Locking the version picker if the scope is `cloud` or `team`**: This change no longer locks the version picker, since there is no way to select a scope. Locking the version picker becomes an issue when Teleport Cloud is a major version behind the latest release. Removing the scope switcher doesn't completely rectify this, since the version of the docs for the previous version includes a warning banner. We can address this in a future change. * Fix formatting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #317
If multiple
Tabscomponents includeTabItems with identical labels, when a user switches to a particularTabItemwithin one of theseTabscomponents, switch the otherTabscomponents as well.This saves a user from clicking the same tabs repeatedly when navigating through the docs site. It also captures most of the usefulness of the scope switcher while being far more visible for users, who tend not to realize that the scope switcher exists. This will make it easier to eventually deprecate the scope switcher.
This change adds a
TabContextProviderhigh in the component tree (just belowDocsContextProvider).Tabscomponents look up their currently selected tab from this context provider, rather than from component-local state. TheTabContextProviderhashes the sorted label names of aTabscomponent to get and set the value of the currently selectedTabItem.Tabscomponents with a dropdown menu, as well asTabscomponents indropDownViewmode, also use this context provider to find the current value of dropdown menu items.Note that the
DocsContextProvider's scope value currently supersedes the values managed by theTabContextProvider. For example, if aTabscomponent on one page has the "Enterprise" dropdown menu item selected, and the user has theossscope selected, aTabscomponent with the same dropdown menu on another page will show "Open Source" instead of "Enterprise". We can change this by deprecating the docs scopes.