tweak: right toc #646
tweak: right toc #646
Conversation
…av component - Added state management to track the active heading based on scroll position. - Introduced an Intersection Observer to dynamically update the active heading as the user scrolls. - Enhanced the generateToc function to highlight the active heading in the table of contents, improving navigation experience.
…tent conditions - Introduced createConditionalToc function to generate a table of contents that respects CustomContent conditions. - Implemented filterRightToc function to filter TOC items based on current context (pageType, cloudPlan, language). - Updated DocTemplate and RightNav components to utilize the new TOC filtering logic, enhancing navigation experience. - Added condition property to TableOfContent interface for better handling of conditional rendering.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @shhdgit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the documentation site's Table of Contents functionality by introducing dynamic filtering based on content conditions and improving user navigation with active scroll highlighting. The changes enable a more personalized and context-aware display of document sections, ensuring users see only the most relevant information in the right-hand navigation. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a conditional table of contents (TOC) feature, enhancing the documentation website by allowing TOC items to be filtered based on platform, plan, and language. It also includes scroll highlighting for the active heading in the right navigation. The changes involve creating a new conditional-toc plugin, modifying the DocTemplate component to filter the TOC, and updating the RightNav component to track the active heading. The code adheres to the provided repository style guide.
| exports.createSchemaCustomization = (options) => { | ||
| createExtraType(options); | ||
| createConditionalToc(options); | ||
| }; |
There was a problem hiding this comment.
It might be beneficial to pass the options object to createExtraType and createConditionalToc separately to avoid potential confusion about which options each function utilizes.
| setTimeout(() => { | ||
| // Observe all heading elements | ||
| headingIds.forEach((id) => { | ||
| const element = document.getElementById(id); | ||
| if (element) { | ||
| observer.observe(element); | ||
| } | ||
| }); | ||
| }, 1000); |
There was a problem hiding this comment.
The setTimeout function is used to delay the execution of the code inside it by 1000ms (1 second). This delay might be necessary to ensure that the elements are rendered before the observer starts observing them. However, a fixed delay might not be reliable in all cases, as the rendering time can vary depending on the device and network conditions. Consider using a more robust approach, such as using a callback function or a promise to ensure that the elements are rendered before the observer starts observing them.
Also, it's generally not recommended to use setTimeout with a magic number like 1000. It would be better to define a constant for the delay and use that instead.
| return items | ||
| .map((item) => { | ||
| // Check if item has condition | ||
| if (item.condition) { | ||
| const { platform, plan, language: conditionLang } = item.condition; | ||
|
|
||
| // Check platform match | ||
| if (platform) { | ||
| const normalizedPlatform = platform.replace("-", ""); | ||
| if (normalizedPlatform !== pageType) { | ||
| return null; // Filter out this item | ||
| } | ||
| } | ||
|
|
||
| // Check plan match | ||
| if (plan) { | ||
| const planArray = plan.split(",").map((p) => p.trim()); | ||
| if (!cloudPlan || !planArray.includes(cloudPlan)) { | ||
| return null; // Filter out this item | ||
| } | ||
| } | ||
|
|
||
| // Check language match | ||
| if (conditionLang) { | ||
| const langArray = conditionLang.split(",").map((lang) => lang.trim()); | ||
| if (!langArray.includes(language)) { | ||
| return null; // Filter out this item | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Recursively filter nested items | ||
| if (item.items && item.items.length > 0) { | ||
| const filteredItems = filterRightToc( | ||
| item.items, | ||
| pageType, | ||
| cloudPlan, | ||
| language | ||
| ); | ||
| return { | ||
| ...item, | ||
| items: filteredItems, | ||
| }; | ||
| } | ||
|
|
||
| return item; | ||
| }) | ||
| .filter((item): item is TableOfContent => item !== null); |
There was a problem hiding this comment.
This function could be simplified by using early returns to reduce nesting and improve readability.
export function filterRightToc(
items: TableOfContent[],
pageType: PageType,
cloudPlan: CloudPlan | null,
language: string
): TableOfContent[] {
if (!items) return [];
return items.map((item) => {
if (!item.condition) {
// No condition, keep the item
return item;
}
const { platform, plan, language: conditionLang } = item.condition;
if (platform) {
const normalizedPlatform = platform.replace("-", "");
if (normalizedPlatform !== pageType) {
return null; // Filter out this item
}
}
if (plan) {
const planArray = plan.split(",").map((p) => p.trim());
if (!cloudPlan || !planArray.includes(cloudPlan)) {
return null; // Filter out this item
}
}
if (conditionLang) {
const langArray = conditionLang.split(",").map((lang) => lang.trim());
if (!langArray.includes(language)) {
return null; // Filter out this item
}
}
// Recursively filter nested items
if (item.items && item.items.length > 0) {
const filteredItems = filterRightToc(
item.items,
pageType,
cloudPlan,
language
);
return {
...item,
items: filteredItems,
};
}
return item;
}).filter((item): item is TableOfContent => item !== null);
}| const { | ||
| createConditionalToc, | ||
| } = require("./gatsby/plugin/conditional-toc/conditional-toc"); |
There was a problem hiding this comment.
Consider adding a comment explaining the purpose of this new plugin.
| } from "shared/utils"; | ||
| import { sliceVersionMark } from "shared/utils/anchor"; | ||
| import { GTMEvent, gtmTrack } from "shared/utils/gtm"; | ||
| import { getPageType } from "shared/utils"; |
There was a problem hiding this comment.
This import is no longer used and can be removed.
| <Trans i18nKey="doc.toc" /> | ||
| </Typography> | ||
| {generateToc(toc)} | ||
| {generateToc(toc, 0, activeId)} |
There was a problem hiding this comment.
Consider adding a key prop to the generateToc component to improve performance.
| let tocItems: TableOfContent[] = []; | ||
| if (toc.items?.length === 1) { | ||
| tocItems = toc.items![0].items || []; | ||
| } else { | ||
| tocItems = toc.items || []; | ||
| } | ||
| return tableOfContents.items || []; | ||
| }, [tableOfContents.items]); | ||
|
|
||
| // Filter TOC based on CustomContent conditions | ||
| return filterRightToc(tocItems, pageType, cloudPlan, language); | ||
| }, [toc.items, pageType, cloudPlan, language]); |
There was a problem hiding this comment.
Consider extracting the logic inside React.useMemo to a separate function for better readability and testability.
close: #643