Open
Conversation
WalkthroughAdds CSS rules to fc-enhanced-tabs: defines a focus-ring width variable, computes padding/margin for vaadin-tab inside vaadin-menu-bar-button based on a Lumo flag, and implements focus-visible styling using an inset box-shadow focus ring with resolved border-radius. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css`:
- Around line 129-136: The CSS removes the native outline on
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring] and relies on
box-shadow which can disappear in forced-colors/high-contrast mode; restore an
accessible visible focus by adding a forced-colors fallback: wrap a rule in
`@media` (forced-colors: active) targeting
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring],
vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible and set an
explicit outline (e.g. outline: 2px solid Highlight or outline: 2px solid
CanvasText) and outline-offset to match the intended ring, or conditionally keep
outline: none only outside forced-colors so the native/high-contrast outline
remains available; update the selectors in the existing block (the
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'] rules and inner
vaadin-menu-bar-item shadow styles) accordingly.
- Around line 109-125: The calc() expressions for padding-block, padding-inline,
margin-block, and margin-inline violate stylelint operator/spacing and have
extraneous blank lines; reformat each declaration so binary operators (+, -, *)
are on the same line as their operands with single spaces around them (or
collapse the entire calc() to a single line) and remove the empty line before
the padding-inline and margin-block declarations so there are no blank lines
between consecutive property declarations; update the padding-block,
padding-inline, margin-block, and margin-inline declarations accordingly to
satisfy stylelint.
Comment on lines
109
to
125
| padding-block: calc( | ||
| (var(--_lumo-flag) * 0.5rem) + | ||
| ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-block-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) | ||
| ); | ||
|
|
||
| padding-inline: calc( | ||
| (var(--_lumo-flag) * 0.5rem) + | ||
| ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-inline-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) | ||
| ); | ||
|
|
||
| margin-block: calc( | ||
| (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-block-container, -9999px) - var(--vaadin-padding-block-container, 0px)) | ||
| ); | ||
|
|
||
| margin-inline: calc( | ||
| (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-inline-container, -9999px) - var(--vaadin-padding-inline-container, 0px)) | ||
| ); |
There was a problem hiding this comment.
Fix calc/operator formatting to pass stylelint.
This block currently violates stylelint rules (operator newline/spacing and empty lines before declarations), which can block CI.
💡 Suggested formatting fix
padding-block: calc(
- (var(--_lumo-flag) * 0.5rem) +
+ (var(--_lumo-flag) * 0.5rem) +
((1 - var(--_lumo-flag)) * (var(--vaadin-padding-block-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width)))
);
-
padding-inline: calc(
- (var(--_lumo-flag) * 0.5rem) +
+ (var(--_lumo-flag) * 0.5rem) +
((1 - var(--_lumo-flag)) * (var(--vaadin-padding-inline-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width)))
);
-
margin-block: calc(
(1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-block-container, -9999px) - var(--vaadin-padding-block-container, 0px))
);
-
margin-inline: calc(
(1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-inline-container, -9999px) - var(--vaadin-padding-inline-container, 0px))
);📝 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.
Suggested change
| padding-block: calc( | |
| (var(--_lumo-flag) * 0.5rem) + | |
| ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-block-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) | |
| ); | |
| padding-inline: calc( | |
| (var(--_lumo-flag) * 0.5rem) + | |
| ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-inline-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) | |
| ); | |
| margin-block: calc( | |
| (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-block-container, -9999px) - var(--vaadin-padding-block-container, 0px)) | |
| ); | |
| margin-inline: calc( | |
| (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-inline-container, -9999px) - var(--vaadin-padding-inline-container, 0px)) | |
| ); | |
| padding-block: calc( | |
| (var(--_lumo-flag) * 0.5rem) + | |
| ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-block-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) | |
| ); | |
| padding-inline: calc( | |
| (var(--_lumo-flag) * 0.5rem) + | |
| ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-inline-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) | |
| ); | |
| margin-block: calc( | |
| (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-block-container, -9999px) - var(--vaadin-padding-block-container, 0px)) | |
| ); | |
| margin-inline: calc( | |
| (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-inline-container, -9999px) - var(--vaadin-padding-inline-container, 0px)) | |
| ); |
🧰 Tools
🪛 Stylelint (17.3.0)
[error] 110-110: Unexpected newline after "+" (scss/operator-no-newline-after)
(scss/operator-no-newline-after)
[error] 115-115: Unexpected newline after "+" (scss/operator-no-newline-after)
(scss/operator-no-newline-after)
[error] 114-117: Unexpected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 119-121: Unexpected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 123-125: Unexpected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 110-110: Expected single space after "+" operator (function-calc-no-unspaced-operator)
(function-calc-no-unspaced-operator)
[error] 115-115: Expected single space after "+" operator (function-calc-no-unspaced-operator)
(function-calc-no-unspaced-operator)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css`
around lines 109 - 125, The calc() expressions for padding-block,
padding-inline, margin-block, and margin-inline violate stylelint
operator/spacing and have extraneous blank lines; reformat each declaration so
binary operators (+, -, *) are on the same line as their operands with single
spaces around them (or collapse the entire calc() to a single line) and remove
the empty line before the padding-inline and margin-block declarations so there
are no blank lines between consecutive property declarations; update the
padding-block, padding-inline, margin-block, and margin-inline declarations
accordingly to satisfy stylelint.
Member
Author
There was a problem hiding this comment.
This is what CodeRabbit reported, but the submitted code follows the current formatting conventions.
There was a problem hiding this comment.
@javier-godoy, understood! Maintaining consistency with the existing formatting conventions in your codebase makes sense. If the stylelint rules don't align with your project's style, you could consider updating the .stylelintrc configuration to match your conventions (e.g., adjusting declaration-empty-line-before, scss/operator-no-newline-after, and function-calc-no-unspaced-operator rules), or disabling those specific rules for this file or project if they're not adding value.
✏️ Learnings added
Learnt from: javier-godoy
Repo: FlowingCode/EnhancedTabs PR: 28
File: src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css:109-125
Timestamp: 2026-03-02T17:42:29.796Z
Learning: In the FlowingCode/EnhancedTabs repository, CSS formatting conventions allow newlines after operators in calc() expressions and empty lines between declaration blocks, even if this conflicts with default stylelint rules.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+129
to
+136
| vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring], | ||
| vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible { | ||
| outline: none; | ||
| vaadin-menu-bar-item { | ||
| box-shadow: inset 0 0 0 var(--focus-ring-width) var(--vaadin-focus-ring-color, var(--lumo-primary-color-50pct)); | ||
| border-radius: var(--lumo-border-radius-m, var(--vaadin-tab-border-radius, var(--vaadin-radius-m))); | ||
| } | ||
| } |
There was a problem hiding this comment.
Keep visible focus in forced-colors mode.
Line 131 removes outline and relies on box-shadow; in forced-colors/high-contrast contexts, that ring may not render reliably, causing keyboard focus loss.
♿ Suggested accessibility fallback
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring],
vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible {
outline: none;
vaadin-menu-bar-item {
box-shadow: inset 0 0 0 var(--focus-ring-width) var(--vaadin-focus-ring-color, var(--lumo-primary-color-50pct));
border-radius: var(--lumo-border-radius-m, var(--vaadin-tab-border-radius, var(--vaadin-radius-m)));
}
}
+
+@media (forced-colors: active) {
+ vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring],
+ vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible {
+ outline: 2px solid CanvasText;
+ outline-offset: 2px;
+ }
+
+ vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring] vaadin-menu-bar-item,
+ vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible vaadin-menu-bar-item {
+ box-shadow: none;
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css`
around lines 129 - 136, The CSS removes the native outline on
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring] and relies on
box-shadow which can disappear in forced-colors/high-contrast mode; restore an
accessible visible focus by adding a forced-colors fallback: wrap a rule in
`@media` (forced-colors: active) targeting
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring],
vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible and set an
explicit outline (e.g. outline: 2px solid Highlight or outline: 2px solid
CanvasText) and outline-offset to match the intended ring, or conditionally keep
outline: none only outside forced-colors so the native/high-contrast outline
remains available; update the selectors in the existing block (the
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'] rules and inner
vaadin-menu-bar-item shadow styles) accordingly.
Member
Author
There was a problem hiding this comment.
@coderabbitai Please create an issue about it.
Component has never rendered reliably in forced-colors mode.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for forced-colors mode accessibility concern
gh issue create \
--title "Improve forced-colors mode accessibility for focus indicators" \
--body "## Description
The EnhancedTabs component does not render reliably in forced-colors (high-contrast) mode. The current focus ring implementation relies on \`box-shadow\` which may not be visible in forced-colors contexts, potentially causing keyboard focus to be invisible to users who rely on high-contrast settings.
## Context
This issue was identified during review of PR `#28`, which addresses focus-ring styles for Vaadin 24/25.
**Related PR:** https://github.com/FlowingCode/EnhancedTabs/pull/28
**Related comment:** https://github.com/FlowingCode/EnhancedTabs/pull/28#discussion_r2872100914
## Expected Behavior
Focus indicators should remain visible and accessible in forced-colors/high-contrast mode to ensure keyboard navigation remains usable for all users.
## Current Behavior
The component has rendering issues in forced-colors mode, including focus indicators that may not be visible.
---
Reported by: `@javier-godoy`" \
--assignee javier-godoy
Length of output: 53
@javier-godoy, I've created issue #29 to track the forced-colors mode accessibility improvements. The issue captures the rendering concerns shown in your screenshot and will allow this to be addressed separately from the current focus-ring fixes in this PR.
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css (2)
129-136:⚠️ Potential issue | 🟠 MajorAdd a forced-colors focus fallback.
Removing outline and relying only on
box-shadowrisks invisible keyboard focus in high-contrast/forced-colors mode.♿ Suggested accessibility fallback
vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring], vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible { outline: none; vaadin-menu-bar-item { box-shadow: inset 0 0 0 var(--focus-ring-width) var(--vaadin-focus-ring-color, var(--lumo-primary-color-50pct)); border-radius: var(--lumo-border-radius-m, var(--vaadin-tab-border-radius, var(--vaadin-radius-m))); } } + +@media (forced-colors: active) { + vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring], + vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible { + outline: 2px solid CanvasText; + outline-offset: 2px; + } + + vaadin-menu-bar-button[theme~='fc-enhanced-tabs'][focus-ring] vaadin-menu-bar-item, + vaadin-menu-bar-button[theme~='fc-enhanced-tabs']:focus-visible vaadin-menu-bar-item { + box-shadow: none; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css` around lines 129 - 136, The focus styling removes outline and relies on box-shadow which can disappear in forced-colors/high-contrast modes; add a forced-colors fallback by targeting the same selector (vaadin-menu-bar-button[theme~='fc-enhanced-tabs'] and its :focus-visible) inside an `@media` (forced-colors: active) block and restore an accessible visible focus (e.g., apply a system-safe outline like outline: 2px solid Highlight with an outline-offset or use forced-colors-aware system color values) so keyboard focus remains visible in high-contrast modes while keeping the current box-shadow for normal themes.
109-125:⚠️ Potential issue | 🟡 MinorReformat
calc()blocks to satisfy stylelint.The current line breaks/blank lines in these declarations still match the reported lint violations and may fail CI.
💡 Suggested formatting fix
padding-block: calc( - (var(--_lumo-flag) * 0.5rem) + + (var(--_lumo-flag) * 0.5rem) + ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-block-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) ); - padding-inline: calc( - (var(--_lumo-flag) * 0.5rem) + + (var(--_lumo-flag) * 0.5rem) + ((1 - var(--_lumo-flag)) * (var(--vaadin-padding-inline-container, calc(0.5rem + var(--focus-ring-width))) - var(--focus-ring-width))) ); - margin-block: calc( (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-block-container, -9999px) - var(--vaadin-padding-block-container, 0px)) ); - margin-inline: calc( (1 - var(--_lumo-flag)) * max(0px, var(--focus-ring-width) + var(--vaadin-padding-inline-container, -9999px) - var(--vaadin-padding-inline-container, 0px)) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css` around lines 109 - 125, Reformat the multi-line calc() declarations for padding-block, padding-inline, margin-block and margin-inline so they follow stylelint's expected single-expression formatting (remove extra blank lines and unnecessary line breaks inside each calc), preserving the same math and CSS variables (e.g., --_lumo-flag, --vaadin-padding-block-container, --focus-ring-width, --vaadin-padding-inline-container) and keep the arithmetic and max(...) expressions intact; update the calc() blocks for padding-block, padding-inline, margin-block and margin-inline to be compact and consistently formatted to satisfy the linter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/resources/META-INF/frontend/fcEnhancedTabs/fc-enhanced-tabs.css`:
- Around line 129-136: The focus styling removes outline and relies on
box-shadow which can disappear in forced-colors/high-contrast modes; add a
forced-colors fallback by targeting the same selector
(vaadin-menu-bar-button[theme~='fc-enhanced-tabs'] and its :focus-visible)
inside an `@media` (forced-colors: active) block and restore an accessible visible
focus (e.g., apply a system-safe outline like outline: 2px solid Highlight with
an outline-offset or use forced-colors-aware system color values) so keyboard
focus remains visible in high-contrast modes while keeping the current
box-shadow for normal themes.
- Around line 109-125: Reformat the multi-line calc() declarations for
padding-block, padding-inline, margin-block and margin-inline so they follow
stylelint's expected single-expression formatting (remove extra blank lines and
unnecessary line breaks inside each calc), preserving the same math and CSS
variables (e.g., --_lumo-flag, --vaadin-padding-block-container,
--focus-ring-width, --vaadin-padding-inline-container) and keep the arithmetic
and max(...) expressions intact; update the calc() blocks for padding-block,
padding-inline, margin-block and margin-inline to be compact and consistently
formatted to satisfy the linter.
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 by CodeRabbit