Add module-level announcements and refactor context switcher to dropdown UI #317
Conversation
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis change removes the separate CSS files for badges and context switcher, introducing a new consolidated Sequence Diagram(s)sequenceDiagram
participant User
participant HandlebarsTemplate
participant Helpers
participant CSS
participant JS
User->>HandlebarsTemplate: Request page render
HandlebarsTemplate->>Helpers: get-module-announcement (for announcement bar)
Helpers-->>HandlebarsTemplate: Announcement data (if any)
HandlebarsTemplate->>Helpers: resolve-resource (for links)
Helpers-->>HandlebarsTemplate: Resolved URLs
HandlebarsTemplate->>CSS: Render metadata, badges, context-switcher styles
CSS-->>HandlebarsTemplate: Apply consolidated styles from metadata.css
HandlebarsTemplate->>JS: Include dropdown and tooltip scripts
JS-->>User: Enable interactive context switcher and metadata toggling
HandlebarsTemplate-->>User: Serve rendered HTML with new styles and logic
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/helpers/concat.js (1)
1-1: Remove redundant 'use strict' directive.The 'use strict' directive is automatically applied to all ES modules and is redundant here.
-'use strict' -src/helpers/get-property.js (2)
1-1: Remove redundant 'use strict' directive.The 'use strict' directive is automatically applied to all ES modules and is redundant here.
-'use strict' -
4-4: Update JSDoc to reflect actual functionality.The comment mentions "nested property" but the implementation only supports single-level property access. Consider updating the comment for accuracy.
- * Gets a nested property from an object using a dynamic key + * Gets a property from an object using a dynamic keysrc/helpers/get-module-announcement.js (1)
1-1: Remove redundant 'use strict' directive.The 'use strict' directive is automatically applied to all ES modules and is redundant here.
-'use strict' -src/partials/article.hbs (1)
46-63: Add safety check for beta-text attribute.The tooltip initialization looks good, but consider adding a safety check for the
beta-textattribute to prevent undefined content in tooltips.if (topBadge && navBadge) { + const betaText = "{{page.attributes.beta-text}}" || "This feature is in beta"; tippy(topBadge, { - content: "{{page.attributes.beta-text}}", + content: betaText, animation: "scale", theme: "redpanda-term", touch: "hold", interactive: true, allowHTML: true, }); tippy(navBadge, { - content: "{{page.attributes.beta-text}}", + content: betaText, animation: "scale", theme: "redpanda-term", touch: "hold", interactive: true, allowHTML: true, }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/css/badges.css(0 hunks)src/css/context-switcher.css(0 hunks)src/css/metadata.css(1 hunks)src/css/site.css(1 hunks)src/helpers/concat.js(1 hunks)src/helpers/get-module-announcement.js(1 hunks)src/helpers/get-property.js(1 hunks)src/partials/announcement-bar.hbs(1 hunks)src/partials/article.hbs(1 hunks)src/partials/component-home-v2.hbs(4 hunks)src/partials/component-home.hbs(2 hunks)src/partials/context-switcher.hbs(1 hunks)src/partials/home.hbs(2 hunks)src/partials/toc.hbs(1 hunks)
💤 Files with no reviewable changes (2)
- src/css/badges.css
- src/css/context-switcher.css
🧰 Additional context used
🪛 Biome (1.9.4)
src/helpers/concat.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/helpers/get-property.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/helpers/get-module-announcement.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (20)
src/helpers/concat.js (1)
9-13: Helper implementation is correct.The logic correctly handles Handlebars helper arguments by removing the context object and concatenating the remaining strings.
src/helpers/get-property.js (1)
10-13: Helper implementation is correct for single-level property access.The validation and property access logic is sound and handles edge cases appropriately.
src/helpers/get-module-announcement.js (1)
10-30: Excellent defensive programming and logic.The implementation correctly validates all required properties before proceeding and handles the dynamic key construction appropriately. The return structure is consistent and provides sensible defaults.
src/css/site.css (1)
8-8: CSS consolidation change looks good.The addition of
metadata.cssimport aligns well with the PR's goal of consolidating badge and context switcher styles into a single file. The positioning in the import order is appropriate.src/partials/component-home-v2.hbs (1)
4-4: URL resolution standardization looks good.The systematic replacement of
resolvePageURLwithresolve-resourcemaintains the existing functionality while improving consistency across templates. All instances correctly preserve therelativizewrapper and URL resolution logic.Also applies to: 17-17, 41-41, 67-67
src/partials/home.hbs (1)
47-47: LGTM! Helper function replacement looks consistent.The replacement of
resolvePageURLwithresolve-resourcefollows the standardization pattern across templates. Both instances maintain proper syntax and structure.Also applies to: 83-83
src/partials/context-switcher.hbs (2)
5-6: Helper function replacement looks consistent.The replacement of
resolvePageURLwithresolve-resourcein both the href attribute and active class condition follows the standardization pattern. The syntax and structure are correct.
2-2: Backward compatibility verified: nocontext-linksusages found.A search for the old
context-linksattribute returned zero matches, and onlycontext-switcherremains across all templates and CSS. The migration is complete and safe to merge.src/partials/component-home.hbs (1)
5-5: LGTM! Consistent helper function standardization.The replacement of
resolvePageURLwithresolve-resourceis correctly implemented in both the feature list and footer menu sections, maintaining proper syntax and structure.Also applies to: 35-35
src/partials/toc.hbs (1)
6-15: Well-structured metadata container consolidation.The new metadata container structure effectively groups the beta label and context switcher into a unified component with consistent styling classes. The conditional rendering logic for both elements is correctly implemented, and the
hiddenclass suggests proper JavaScript-controlled visibility.src/css/metadata.css (2)
1-154: Well-structured CSS consolidation with comprehensive styling.The new metadata.css file effectively consolidates badge and context switcher styles into a cohesive system. The code demonstrates good practices:
- Clear BEM-like naming conventions
- Proper use of flexbox for layout
- Smooth transitions for state changes
- Responsive design considerations
- Consistent theming approach
19-19: All referenced CSS custom properties are definedI’ve verified that each of the variables used in
src/css/metadata.cssis declared in the global styles:
--body-backgroundin
• src/css/vars.css
• src/css/dark-mode.css--link-highlight-colorin
• src/css/vars.css
• src/css/search-bump.css
• src/css/dark-mode.css
• src/css/doc-bump.css--beta-label-background&--beta-label-colorin src/css/vars.css--link-highlight-background-colorin
• src/css/vars.css
• src/css/dark-mode.css
• src/css/search-bump.cssNo further action required.
src/partials/article.hbs (4)
19-28: LGTM! Clean consolidation of metadata elements.The new unified
.metadata--maincontainer effectively consolidates the beta label and context switcher elements, improving the structure and maintainability. The conditional rendering logic is correct and the HTML structure is semantic.
29-29: Good performance optimization.Conditionally loading the script only when metadata features are present reduces unnecessary JavaScript execution on pages without beta labels or context switchers.
32-37: Excellent defensive programming.The null checks with early return prevent potential runtime errors if the expected metadata containers are not present. The variable naming clearly reflects the new container structure.
65-79: Improved visibility toggling with better UX.The IntersectionObserver implementation correctly toggles the navigation metadata visibility based on the main metadata container's visibility. The reduced threshold (0.1) provides earlier triggering, improving the user experience.
src/partials/announcement-bar.hbs (4)
1-1: Comprehensive announcement source handling.The extended conditional logic properly covers all announcement sources (page-level, component version, module-level, and site-wide) while correctly excluding API component pages. The logic structure is well-organized and uses appropriate Handlebars helpers.
3-3: Well-structured announcement key generation.The data-announcement-key attribute generates appropriately unique identifiers for different announcement types, incorporating relevant context (component, version, module, path) for effective tracking and caching.
8-21: Well-structured announcement priority system.The announcement content rendering follows a logical priority order (page → component → module → site-wide) with proper fallback handling. The use of the
withblock for module announcements correctly handles the helper result.
7-18: Consistent URL resolution pattern implemented.All announcement links now use the unified
resolve-resourceandrelativizepattern, ensuring consistent URL handling across different announcement types. The triple braces correctly prevent HTML escaping of the generated URLs.
| // For preview builds where contentCatalog.resolveResource might not exist | ||
| if (!contentCatalog || !contentCatalog.resolveResource) { | ||
| // Return the resource as-is for preview builds | ||
| return resource |
paulohtb6
left a comment
There was a problem hiding this comment.
lgtm. Just be mindful of the css class name
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/helpers/resolve-resource.js (1)
3-3: Add null/undefined check before usingresourcein resolve-resource helperThe helper currently assumes
resourceis a string, so callingresource.startsWithwill throw if it’snullorundefined. Add an early guard to return as-is whenresourceis falsy:• File:
src/helpers/resolve-resource.js
• Location: directly above the existingif (resource.startsWith('http')) { … }Suggested diff:
module.exports = (resource, { data, hash: context }) => { + if (resource == null) { + return resource + } if (resource.startsWith('http')) { return resource } // … }
🧹 Nitpick comments (2)
src/helpers/is-eol.js (1)
18-19: Consider using more precise leap year handling for EOL calculation.The hardcoded 365-day threshold doesn't account for leap years, which could cause slight inaccuracies in EOL determination. Consider using a more precise approach:
- // EOL if one year (365 days) or more has passed - const msInYear = 365 * 24 * 60 * 60 * 1000 - return (now - release) >= msInYear + // EOL if one year or more has passed (accounting for leap years) + const oneYearLater = new Date(release) + oneYearLater.setFullYear(oneYearLater.getFullYear() + 1) + return now >= oneYearLaterThis approach automatically handles leap years and daylight saving time transitions correctly.
src/partials/context-switcher.hbs (1)
75-78: Consider reducing or removing the setTimeout delay.The 100ms delay before focusing the first item seems arbitrary and could impact user experience. Consider either removing it or using a smaller delay:
- // Focus first item when opening - setTimeout(function() { - const firstItem = menu.querySelector('.context-dropdown-item'); - if (firstItem) firstItem.focus(); - }, 100); + // Focus first item when opening + const firstItem = menu.querySelector('.context-dropdown-item'); + if (firstItem) firstItem.focus();If the delay is necessary for layout reasons, consider using
requestAnimationFrameinstead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/css/metadata.css(1 hunks)src/css/nav.css(7 hunks)src/helpers/is-eol.js(1 hunks)src/helpers/resolve-resource.js(1 hunks)src/js/01-nav.js(0 hunks)src/js/03-fragment-jumper.js(0 hunks)src/partials/context-switcher.hbs(1 hunks)src/partials/nav-explore.hbs(1 hunks)
💤 Files with no reviewable changes (2)
- src/js/03-fragment-jumper.js
- src/js/01-nav.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/css/metadata.css
🔇 Additional comments (10)
src/helpers/is-eol.js (1)
6-12: LGTM: Good defensive programming with explicit EOL handling.The function correctly handles both object and string inputs, with proper fallback logic from explicit
page-eolattributes to release dates.src/css/nav.css (3)
2-3: LGTM: Consistent padding reduction for tighter layout.The reduced padding values create a more compact navigation layout while maintaining readability.
322-323: Good use of modern CSS gap property.Replacing
align-items: centerwithgap: 2pxprovides better control over spacing between flex items and is more semantically appropriate for this layout.
55-58: No layout gaps or conflicts with the updated breakpointsThe media-query ranges in nav.css are contiguous and properly scoped:
- 769px–1023px applies only to
.nav-panel-explore- ≥1024px rules take over for
.nav-containerand other elements- There is no overlap or missing range between these queries
No further changes needed here.
src/helpers/resolve-resource.js (2)
9-13: Excellent guard clause for preview build compatibility.The addition of checks for
contentCatalogandcontentCatalog.resolveResourcewith fallback to the original resource ensures robust behavior in preview environments where the content catalog might not be fully initialized.
19-19: Good fallback handling for missing files.The fallback to the original resource when
fileis not found prevents returningundefinedand maintains consistent return types. This addresses edge cases where resources might not exist in the catalog.src/partials/nav-explore.hbs (2)
5-13: Excellent integration of EOL detection for current version.The logic correctly iterates through versions to find the current one and displays the EOL label when appropriate. The conditional rendering ensures the label only appears when needed.
20-27: LGTM: Consistent EOL labeling and improved URL resolution.The dropdown items now use
resolve-resourcefor better URL handling and display EOL labels consistently with the current version. The existingis-missingclass logic is preserved.src/partials/context-switcher.hbs (2)
1-25: Excellent transformation to accessible dropdown component.The new markup properly implements ARIA roles and attributes, with clean separation between the toggle button and menu items. The use of
resolve-resourcefor URL resolution aligns with the broader refactoring effort.
82-117: Comprehensive keyboard navigation implementation.The keyboard event handling covers all essential keys (Escape, Arrow keys, Enter, Space) with proper preventDefault usage and focus management. This ensures excellent accessibility compliance.
| // Close all dropdowns when clicking outside - use setTimeout to avoid immediate firing | ||
| document.addEventListener('click', function(e) { | ||
| setTimeout(function() { | ||
| let clickedInsideAny = false; | ||
| dropdowns.forEach(function(dropdown) { | ||
| if (dropdown.contains(e.target)) { | ||
| clickedInsideAny = true; | ||
| } | ||
| }); | ||
| if (!clickedInsideAny) { | ||
| dropdowns.forEach(function(dropdown) { | ||
| const toggle = dropdown.querySelector('.context-dropdown-toggle'); | ||
| const menu = dropdown.querySelector('.context-dropdown-menu'); | ||
| if (toggle && menu) { | ||
| toggle.setAttribute('aria-expanded', 'false'); | ||
| menu.classList.remove('show'); | ||
| } | ||
| }); | ||
| } | ||
| }, 10); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential race condition in click-outside handler.
The setTimeout with arbitrary 10ms delay in the click-outside handler could create race conditions. Consider using a more deterministic approach:
- // Close all dropdowns when clicking outside - use setTimeout to avoid immediate firing
- document.addEventListener('click', function(e) {
- setTimeout(function() {
- let clickedInsideAny = false;
- dropdowns.forEach(function(dropdown) {
- if (dropdown.contains(e.target)) {
- clickedInsideAny = true;
- }
- });
-
- if (!clickedInsideAny) {
- dropdowns.forEach(function(dropdown) {
- const toggle = dropdown.querySelector('.context-dropdown-toggle');
- const menu = dropdown.querySelector('.context-dropdown-menu');
- if (toggle && menu) {
- toggle.setAttribute('aria-expanded', 'false');
- menu.classList.remove('show');
- }
- });
- }
- }, 10);
- });
+ // Close all dropdowns when clicking outside
+ document.addEventListener('click', function(e) {
+ let clickedInsideAny = false;
+ dropdowns.forEach(function(dropdown) {
+ if (dropdown.contains(e.target)) {
+ clickedInsideAny = true;
+ }
+ });
+
+ if (!clickedInsideAny) {
+ dropdowns.forEach(function(dropdown) {
+ const toggle = dropdown.querySelector('.context-dropdown-toggle');
+ const menu = dropdown.querySelector('.context-dropdown-menu');
+ if (toggle && menu) {
+ toggle.setAttribute('aria-expanded', 'false');
+ menu.classList.remove('show');
+ }
+ });
+ }
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Close all dropdowns when clicking outside - use setTimeout to avoid immediate firing | |
| document.addEventListener('click', function(e) { | |
| setTimeout(function() { | |
| let clickedInsideAny = false; | |
| dropdowns.forEach(function(dropdown) { | |
| if (dropdown.contains(e.target)) { | |
| clickedInsideAny = true; | |
| } | |
| }); | |
| if (!clickedInsideAny) { | |
| dropdowns.forEach(function(dropdown) { | |
| const toggle = dropdown.querySelector('.context-dropdown-toggle'); | |
| const menu = dropdown.querySelector('.context-dropdown-menu'); | |
| if (toggle && menu) { | |
| toggle.setAttribute('aria-expanded', 'false'); | |
| menu.classList.remove('show'); | |
| } | |
| }); | |
| } | |
| }, 10); | |
| }); | |
| // Close all dropdowns when clicking outside | |
| document.addEventListener('click', function(e) { | |
| let clickedInsideAny = false; | |
| dropdowns.forEach(function(dropdown) { | |
| if (dropdown.contains(e.target)) { | |
| clickedInsideAny = true; | |
| } | |
| }); | |
| if (!clickedInsideAny) { | |
| dropdowns.forEach(function(dropdown) { | |
| const toggle = dropdown.querySelector('.context-dropdown-toggle'); | |
| const menu = dropdown.querySelector('.context-dropdown-menu'); | |
| if (toggle && menu) { | |
| toggle.setAttribute('aria-expanded', 'false'); | |
| menu.classList.remove('show'); | |
| } | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/partials/context-switcher.hbs around lines 120 to 141, the click-outside
handler uses a setTimeout with a 10ms delay, which can cause race conditions.
Replace the setTimeout with a more deterministic approach such as using event
capturing or checking event propagation phases to ensure the dropdown close
logic runs after other click handlers without arbitrary delays. This will
prevent timing issues and improve reliability.
| {{#if (is-eol ver.asciidoc.attributes)}} | ||
| <span class="eol-label">(end of life)</span> | ||
| {{/if}} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding ARIA labels for better accessibility.
The EOL labels should have proper ARIA attributes to ensure screen readers announce them appropriately.
- {{#if (is-eol ver.asciidoc.attributes)}}
- <span class="eol-label">(end of life)</span>
- {{/if}}
+ {{#if (is-eol ver.asciidoc.attributes)}}
+ <span class="eol-label" aria-label="End of life version">(end of life)</span>
+ {{/if}}Apply similar changes to the dropdown EOL labels for consistency.
Also applies to: 23-25
🤖 Prompt for AI Agents
In src/partials/nav-explore.hbs around lines 8 to 10 and also lines 23 to 25,
the EOL labels lack ARIA attributes for accessibility. Update the <span>
elements containing the EOL labels to include appropriate ARIA attributes such
as role="note" or aria-label to ensure screen readers announce them properly.
Apply the same ARIA enhancements to the dropdown EOL labels for consistency.
This PR enhances the documentation UI with two improvements: module-level announcement support and a complete refactor of the context switcher from a basic list to an accessible dropdown interface.
2025-07-22_12-12-27.mp4
Context Switcher UX Improvements
Problem: The previous context switcher implementation used a simple list layout that didn't clearly indicate its interactive nature or provide adequate visual hierarchy for users navigating between different contexts (versions, environments, etc.).
Solution: Refactored to a modern dropdown interface that provides:
Improved Discoverability: Users can immediately identify the current context and see that alternatives are available
Module-Level Announcements
Problem: The existing announcement system only supported site-wide and component-level announcements, limiting the ability to communicate module-specific information.
Solution: Extended the announcement system to support module-level targeting:
CSS Consolidation and Refactoring
badges.cssandcontext-switcher.cssand consolidated their styles intometadata.css, simplifying the CSS structure and reducing redundancy (src/css/badges.css[1]src/css/context-switcher.css[2]src/css/metadata.css[3].site.cssto reflect the removal ofbadges.cssandcontext-switcher.cssand the addition ofmetadata.css(src/css/site.css[1] [2].New Handlebars Helpers
concat.jsto concatenate multiple strings, simplifying string handling in templates (src/helpers/concat.jssrc/helpers/concat.jsR1-R13).get-module-announcement.jsto retrieve module-specific announcements dynamically (src/helpers/get-module-announcement.jssrc/helpers/get-module-announcement.jsR1-R30).get-property.jsto access nested object properties using dynamic keys (src/helpers/get-property.jssrc/helpers/get-property.jsR1-R13).Template Enhancements
announcement-bar.hbsto use the newget-module-announcementhelper for module-specific announcements and improved link resolution (src/partials/announcement-bar.hbssrc/partials/announcement-bar.hbsL1-R20).article.hbsto use the consolidated metadata structure for beta labels and context switchers, improving maintainability and adding dynamic visibility toggling (src/partials/article.hbs[1] [2].component-home-v2.hbs,component-home.hbs,home.hbs,context-switcher.hbs, andtoc.hbs) to replaceresolvePageURLwithresolve-resourcefor more consistent resource resolution [1] [2] [3] [4] [5] [6] [7] [8] [9] [10].These changes collectively enhance the maintainability of the codebase, improve the consistency of styles and templates, and introduce reusable utilities for dynamic content handling.