-
Notifications
You must be signed in to change notification settings - Fork 159
docs: dropdown reference #8602
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.
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
base: main
Are you sure you want to change the base?
docs: dropdown reference #8602
Conversation
|
|
can take another look if we want to bring back the "taller" text padding but not sure if really needed.. 🤔 |
ericpgreen2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall Summary
Good progress consolidating Reference content into a dropdown. A few content and design items to address before this is ready.
Content & IA
1. docs/docusaurus.config.js (dropdown items)
Missing items and label change:
- Add "Time Syntax" (
/reference/rill-iso-extensions) to the dropdown—it's reference content that's currently missing - Rename "API" to "REST API" for clarity
Design
2. docs/src/css/_navbar.scss (active state styling)
Layout shift issue: The current font-weight: 600 for active items causes adjacent nav items to shift when clicking between them. Options:
| Option | Implementation | Trade-off |
|---|---|---|
| Underline only | Remove bold, keep ::after underline |
No shift, but you mentioned underline + caret felt noisy |
| Color only | Remove bold entirely, rely on color | Minimal, but may feel too subtle |
| Fixed width | Set explicit width on nav items | Prevents shift, but may look awkward |
| Pseudo-element trick | Add invisible bold text via ::before to reserve space |
Preserves current look, no shift |
3. docs/src/css/_navbar.scss (navbar and dropdown styling)
Design polish:
- Top nav padding: The top navbar items have inconsistent padding/widths on hover. Audit and standardize.
- Dropdown gap: Add
margin-top: 4pxto.dropdown__menufor breathing room between trigger and popover. - Dropdown size: Menu items feel oversized. Consider tightening padding from
0.5rem 0.75remto0.375rem 0.75rem. - Caret: The default Docusaurus down-arrow looks dated. Consider replacing with a Rill-style chevron.
Code Cleanup
4. docs/src/theme/Navbar/index.js (lines 54-101)
Two MutationObservers: Both useEffect hooks watch document.body with identical options. Consider consolidating into a single observer.
5. docs/src/css/_navbar.scss (various)
Minor cleanup:
- Lines 17-19: Duplicate comment ("Active navbar item" / "Active navbar link"). Remove the first.
- Line 107: Use
0.5pxinstead of.5pxfor consistency. Consider a CSS variable for the hardcoded#e5e7eb. - Lines 227-230: Dead selector
.mobile-nav-icon-links—the JS was removed, so this targets nothing.
Developed in collaboration with Claude Code
|
|
I think thats everything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-Review Summary
Good progress on the previous feedback—most items have been addressed. However, there are a few UX and behavior issues to resolve before this is ready.
Must Fix
1. Dropdown doesn't close when clicking an item
The dropdown stays open after clicking a menu item. Standard dropdown UX is: click item → navigate → dropdown closes.
This appears to be because Docusaurus dropdowns are hover-based, and the ::before pseudo-element (hover bridge) keeps the dropdown open if the mouse is still in that area after clicking.
Fix: Add logic to close the dropdown on navigation. Options:
- Listen for route changes via
useLocationin the Navbar wrapper and programmatically close dropdowns - Swizzle
DropdownNavbarItemto add anonClickhandler that closes the dropdown
2. Clicking REST API causes page flicker
Steps to reproduce:
- Navigate to Project Files
- Refresh the page
- Click "REST API" in the dropdown
The page flickers/jumps before navigating. This may be related to the dropdown close issue or a navigation race condition.
3. Active state and hover styling should match main
- Underline feels thin: The current
text-decoration-thickness: 1pxwithtext-underline-offset: 14pxappears thinner than the original styling onmain. Consider matching the previous approach. - Hover background too tall: The hover state background extends beyond what's on
main. Audit and match the existing hover styling. - Hover background has different x-padding for the different items.
Code Cleanup
4. docs/src/css/_navbar.scss — Duplicate declarations
Lines 46-47 and 122-124 have duplicate text-decoration-skip-ink: none;. Remove the duplicates.
5. docs/src/theme/Navbar/index.js — Document the setTimeout workaround
The multiple setTimeout calls (100ms, 500ms, 1000ms) at lines 159-161 are a timing workaround. Add a brief comment explaining why these delays are necessary for future maintainers.
Developed in collaboration with Claude Code
|
as you thought, had to swizzle the component. Did so in this commit and made CSS changes as req |


requested: #8450
loom.com/share/a69eb3af381a44bbb9ba028c9f3a6b3a?from_recorder=1&focus_title=1
Some changes:
Checklist: