Uh oh!
There was an error while loading. Please reload this page.
fix: fix focus-ring style in vaadin 24/25 - #28
Merged
Merged
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 |
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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