-
Notifications
You must be signed in to change notification settings - Fork 6
Add styles for inline badges #304
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?
Conversation
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughA new CSS file for badge styling has been introduced, providing base and modifier classes for different badge types and sizes. The site-wide CSS imports have been updated to include this new stylesheet. Cursor styles for certain beta label elements have been changed from pointer to help. Tooltip styles have been updated to enforce consistent font size within headings. Tooltip initialization in JavaScript now includes a guard for the Sequence Diagram(s)sequenceDiagram
participant User
participant Page
participant BadgeCSS
participant TooltipJS
participant TippyLib
User->>Page: Loads page
Page->>BadgeCSS: Loads badge styles via site.css
Page->>TooltipJS: Executes tooltip activation script
TooltipJS->>TippyLib: Checks if tippy is a function
alt tippy is a function
TooltipJS->>TippyLib: Initialize tooltips for [data-tippy-content]
TooltipJS->>TippyLib: Initialize tooltips for [data-tooltip]
end
Page->>User: Renders badges and tooltips with dynamic content
Assessment against linked issues
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 warn ERESOLVE overriding peer dependency ✨ Finishing Touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/css/badges.css (2)
1-7
: Consider setting display and alignment on base badge
Currently.badge
relies on its host element’s display. For consistent box behavior (vertical centering, padding), you may want to add:.badge { display: inline-flex; align-items: center; justify-content: center; /* existing styles… */ }
17-21
: Use CSS variables for fallback badge colors
The hard-coded#e0e0e0
and#1f1f1f
reduce theme flexibility. Consider introducing--badge-label-background
and--badge-label-color
variables so site-wide theming can override these defaults.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/css/badges.css
(1 hunks)src/css/context-switcher.css
(2 hunks)src/css/site.css
(1 hunks)src/css/tooltips.css
(1 hunks)src/js/12-activate-tooltips.js
(1 hunks)src/partials/article.hbs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (8)
src/css/tooltips.css (1)
11-19
: Enforce consistent tooltip font size in headings
The new rule correctly targets popper roots inside all heading levels and uses!important
to override inline styles from Tippy. This ensures tooltips match body copy sizing for readability.src/css/badges.css (2)
9-15
: Large badge variant is well scoped
The.badge--large
rules properly ensure minimum size and centering. Usinginline-flex
here is ideal for icon or text alignment.
30-33
: Large beta badge colors are correct
The.badge--large.badge--beta
selector properly applies the intended background and text colors via--beta-label-*
variables, matching design tokens.src/css/site.css (1)
8-8
: Import badges.css for global badge styling
The import location (afterbody.css
) integrates the badge definitions site-wide. Path and order appear correct.src/css/context-switcher.css (2)
16-18
: Change beta-label hover cursor to help
Switching frompointer
tohelp
better communicates informational intent for beta badges.
58-58
: Apply help cursor to nav-beta-label
Usingcursor: help;
on the navigation badge aligns UX consistency with other beta indicators.src/js/12-activate-tooltips.js (2)
6-6
: Excellent defensive programming practice!The guard clause prevents runtime errors if the tippy library fails to load. This is a good defensive programming approach.
8-15
: Good code organization with helpful comments.The comment clarifies the purpose of this initialization block, making the code more maintainable.
/* Beta */ | ||
.badge--beta { | ||
color: var(--beta-label-background); | ||
font-weight: bold; | ||
background-color: var(--link-highlight-background-color); | ||
} |
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.
Inconsistent CSS variable usage for small beta badge
The small .badge--beta
is using var(--beta-label-background)
for text and var(--link-highlight-background-color)
for its background, but the large variant flips those. This likely inverts your intended palette.
A fix could be:
.badge--beta {
- color: var(--beta-label-background);
- background-color: var(--link-highlight-background-color);
+ color: var(--beta-label-color);
+ background-color: var(--beta-label-background);
font-weight: bold;
}
📝 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.
/* Beta */ | |
.badge--beta { | |
color: var(--beta-label-background); | |
font-weight: bold; | |
background-color: var(--link-highlight-background-color); | |
} | |
/* Beta */ | |
.badge--beta { | |
color: var(--beta-label-color); | |
background-color: var(--beta-label-background); | |
font-weight: bold; | |
} |
🤖 Prompt for AI Agents
In src/css/badges.css between lines 23 and 28, the small .badge--beta uses
inconsistent CSS variables for color and background compared to the large
variant, causing a palette inversion. To fix this, swap the CSS variables so
that the text color uses var(--link-highlight-background-color) and the
background uses var(--beta-label-background), aligning the small badge styling
with the large variant.
"{{page.attributes.beta-text}}", | ||
animation: "scale", |
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.
Sanitize dynamic beta-text to prevent XSS
Injecting page.attributes.beta-text
into tippy(..., allowHTML: true)
without sanitization exposes an XSS vector if that attribute contains malicious HTML. Ensure the source is trusted or run it through a sanitizer. If only plain text is needed, toggle off allowHTML
.
Also applies to: 42-43
🤖 Prompt for AI Agents
In src/partials/article.hbs around lines 33-34 and 42-43, the dynamic insertion
of page.attributes.beta-text into tippy with allowHTML: true poses an XSS risk.
To fix this, either sanitize the beta-text content before passing it to tippy or
disable allowHTML if only plain text is required. Ensure that any HTML content
is from a trusted source or properly sanitized to prevent injection attacks.
// Initialize tippy for custom data-tooltip elements | ||
document.querySelectorAll('[data-tooltip]').forEach((el) => { | ||
tippy(el, { | ||
content: el.getAttribute('data-tooltip'), | ||
animation: 'scale', | ||
theme: 'redpanda-term', | ||
touch: 'hold', | ||
interactive: true, | ||
allowHTML: true, | ||
}) | ||
}) |
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.
🛠️ Refactor suggestion
Potential duplicate initialization issue and code duplication.
The new custom tooltip functionality is well-implemented, but there are a couple of concerns:
-
Duplicate initialization: If an element has both
data-tippy-content
anddata-tooltip
attributes, tippy will be initialized twice on the same element, which could cause unexpected behavior. -
Configuration duplication: The tooltip configuration is duplicated across both initialization blocks.
Consider this refactor to address both issues:
- // Initialize tippy for elements with built-in data-tippy-content
- tippy('[data-tippy-content]', {
- animation: 'scale',
- theme: 'redpanda-term',
- touch: 'hold',
- interactive: true,
- allowHTML: true,
- })
-
- // Initialize tippy for custom data-tooltip elements
- document.querySelectorAll('[data-tooltip]').forEach((el) => {
- tippy(el, {
- content: el.getAttribute('data-tooltip'),
- animation: 'scale',
- theme: 'redpanda-term',
- touch: 'hold',
- interactive: true,
- allowHTML: true,
- })
- })
+ // Shared tooltip configuration
+ const tooltipConfig = {
+ animation: 'scale',
+ theme: 'redpanda-term',
+ touch: 'hold',
+ interactive: true,
+ allowHTML: true,
+ }
+
+ // Initialize tippy for elements with built-in data-tippy-content
+ tippy('[data-tippy-content]:not([data-tooltip])', tooltipConfig)
+
+ // Initialize tippy for custom data-tooltip elements
+ document.querySelectorAll('[data-tooltip]').forEach((el) => {
+ tippy(el, {
+ ...tooltipConfig,
+ content: el.getAttribute('data-tooltip'),
+ })
+ })
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/js/12-activate-tooltips.js around lines 17 to 27, there is a risk of
initializing tippy twice on elements that have both data-tippy-content and
data-tooltip attributes, and the tooltip configuration is duplicated in both
initialization blocks. To fix this, consolidate the initialization logic by
merging the selectors and applying a single tippy initialization with a shared
configuration object. Use a conditional to determine the content source
(data-tippy-content or data-tooltip) to avoid duplicate initialization and
remove redundant configuration code.
This pull request introduces enhancements to the user interface and tooltip functionality, including new badge styles, improved tooltip initialization, and dynamic beta text for tooltips. The changes are grouped into three themes: badge styles, tooltip improvements, and dynamic beta text.
Badge Styles:
badges.css
file with base styles for badges, including variations for large badges and beta badges. These styles improve consistency and usability across the application. (src/css/badges.css
, src/css/badges.cssR1-R33)site.css
to import the newbadges.css
file, ensuring the badge styles are applied globally. (src/css/site.css
, src/css/site.cssR8)Tooltip Improvements:
tooltips.css
to ensure consistent font sizes for tooltips in headers (h1
throughh6
). (src/css/tooltips.css
, src/css/tooltips.cssR11-R19)12-activate-tooltips.js
to support custom tooltips usingdata-tooltip
attributes, alongside the existingdata-tippy-content
attributes. (src/js/12-activate-tooltips.js
, src/js/12-activate-tooltips.jsL6-R27)Dynamic Beta Text:
article.hbs
to use dynamic beta text frompage.attributes.beta-text
, allowing for customizable beta feature descriptions in tooltips. (src/partials/article.hbs
, [1] [2]