fix(docs): broken link and add split Get Started with audience links #39467
Open
shantanukhond wants to merge 1 commit intoapache:masterfrom
Open
fix(docs): broken link and add split Get Started with audience links #39467shantanukhond wants to merge 1 commit intoapache:masterfrom
shantanukhond wants to merge 1 commit intoapache:masterfrom
Conversation
…n hero and navbar
Contributor
Code Review Agent Run #e283e9Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Author
|
If any changes or suggestions required you can let me know. Thank you |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Comment on lines
+150
to
+152
| const mainLink = button.querySelector('a.split-main'); | ||
| const href = | ||
| mainLink?.getAttribute('href') || button.getAttribute('href') || ''; |
Contributor
There was a problem hiding this comment.
Suggestion: The new CTA tracking logic always falls back to the split button's main link, so clicking the chevron trigger (which only opens the dropdown) is incorrectly recorded as a /user-docs/ click. Use the actually clicked anchor first, and only fall back to the element's own href, so analytics reflects real navigation targets. [logic error]
Severity Level: Major ⚠️
- ⚠️ Matomo CTA metrics misreport dropdown-trigger clicks as /user-docs/.
- ⚠️ Homepage Get Started menu usage statistics become inaccurate.
- ⚠️ Navbar Get Started split-button interactions tracked with wrong targets.
Suggested change
| const mainLink = button.querySelector('a.split-main'); | |
| const href = | |
| mainLink?.getAttribute('href') || button.getAttribute('href') || ''; | |
| const clickedLink = event.target.closest?.('a'); | |
| const href = | |
| clickedLink?.getAttribute('href') || button.getAttribute('href') || ''; |
Steps of Reproduction ✅
1. Start the docs site using `cd docs && yarn start` as described in the PR, which serves
the homepage from `docs/src/pages/index.tsx`.
2. On the homepage, locate the hero "Get Started" split button rendered by
`GetStartedSplitButton` at `docs/src/pages/index.tsx:707-710`, which uses the component
from `docs/src/components/GetStartedSplitButton.tsx:124-135` (main link `<Link
to="/user-docs/" className="split-main">` at line 132 and dropdown trigger `<button
className="split-dropdown-trigger">` at lines 25-32).
3. Click directly on the chevron dropdown trigger (the button with
`className="split-dropdown-trigger"`) in the hero, not on the "Get Started" text. This
click is inside the `.default-button-theme` container
(`rootClassName="default-button-theme get-started-split"` in `index.tsx:709`), so the
global click handler registered in `docs/src/theme/Root.js` at lines 284-287
(`document.addEventListener('click', handleCTAClick);`) runs.
4. In `handleCTAClick` at `docs/src/theme/Root.js:146-75`, `button` becomes the outer
`.default-button-theme` element, `mainLink = button.querySelector('a.split-main')` finds
the `/user-docs/` anchor, and `href` is set to that `/user-docs/` URL even though the
chevron click only opens the `Dropdown` (no navigation). As a result, `trackEvent('CTA',
'Click', `${buttonText} - ${href}`);` logs the event as a `/user-docs/` CTA click. The
same mis-attribution occurs when clicking the navbar "Get Started" chevron rendered via
`docs/src/theme/NavbarItem/GetStartedSplitNavbarItem.tsx:23-26` (root class
`default-button-theme get-started-split get-started-button`), since the handler logic is
identical. The suggested change to derive `href` from `event.target.closest('a')` would
correctly leave `href` empty for chevron clicks, matching the fact that no navigation
occurs.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/src/theme/Root.js
**Line:** 150:152
**Comment:**
*Logic Error: The new CTA tracking logic always falls back to the split button's main link, so clicking the chevron trigger (which only opens the dropdown) is incorrectly recorded as a `/user-docs/` click. Use the actually clicked anchor first, and only fall back to the element's own `href`, so analytics reflects real navigation targets.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| * under the License. | ||
| */ | ||
| import GetStartedSplitButton from '../../components/GetStartedSplitButton'; | ||
|
|
Contributor
There was a problem hiding this comment.
Suggestion: The split button relies on classes defined in main.css, but that stylesheet is only imported on the homepage, so opening any non-homepage route directly renders an unstyled/broken navbar control. Import the stylesheet in this navbar item (or the shared button component) so styles are always loaded wherever the navbar renders. [possible bug]
Severity Level: Major ⚠️
- ⚠️ Navbar Get Started button unstyled on deep-linked docs pages.
- ⚠️ Dropdown menu for Get Started lacks intended gradient theming.
- ⚠️ Inconsistent hero vs navbar CTA appearance across documentation.
Suggested change
| import '../../styles/main.css'; |
Steps of Reproduction ✅
1. Start the docs site using the PR instructions: from `/workspace/superset/docs`, run
`yarn start` (or `yarn start:quick`) so Docusaurus serves the documentation.
2. In a fresh browser session, directly open a non-homepage docs route, for example
`http://localhost:3000/user-docs/using-superset/creating-your-first-dashboard` (this route
is linked from `docs/quickstart.mdx:76` via
`/user-docs/using-superset/creating-your-first-dashboard`).
3. Docusaurus config at `docs/docusaurus.config.ts:895-898` adds a navbar item with `type:
'custom-getStartedSplit'`, which is resolved by
`docs/src/theme/NavbarItem/ComponentTypes.tsx:30-40` to `GetStartedSplitNavbarItem`,
implemented in `docs/src/theme/NavbarItem/GetStartedSplitNavbarItem.tsx:19-26` and
rendering `GetStartedSplitButton` with `rootClassName="default-button-theme
get-started-split get-started-button"`.
4. The styles for `.default-button-theme`, `.get-started-split`, the dropdown menu, and
navbar-specific sizing are defined only in
`docs/src/styles/main.css:74-165,114-141,149-165`, which is imported exclusively by the
homepage component `docs/src/pages/index.tsx:31-32` (`import '../styles/main.css';`).
Because the `/user-docs/...` route does not load `index.tsx`, `main.css` is not included
on initial render, so the navbar "Get Started" split button appears without its gradient
background, sizing, and dropdown theming until the homepage is visited, demonstrating that
the navbar component relies on CSS that is not loaded on non-homepage routes.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/src/theme/NavbarItem/GetStartedSplitNavbarItem.tsx
**Line:** 20:20
**Comment:**
*Possible Bug: The split button relies on classes defined in `main.css`, but that stylesheet is only imported on the homepage, so opening any non-homepage route directly renders an unstyled/broken navbar control. Import the stylesheet in this navbar item (or the shared button component) so styles are always loaded wherever the navbar renders.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
cd docs && yarn startoryarn start:quick./user-docs/. Click the chevron → menu with Users, Admins, Developers; each link goes to/user-docs/,/admin-docs/,/developer-docs/respectively.ADDITIONAL INFORMATION