Skip to content

fix(mixins): improve focus-visible visualisation #4022

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.

Sign up for GitHub

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

Open
LucyChyzhova wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(mixins): improve focus-visible visualisation #4022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/style/mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
}

&:focus-visible {
outline: none;
Copy link
Copy Markdown
Contributor

@Kiarokh Kiarokh Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick bad not… just writing so we don't forget. Many components don't use our "clickable" mixins, and replicate them. They also add transitions for box-shadow. Now that we are changing this to outline, we need to make sure that those transitions are updated if necessary. I'll elaborate later

box-shadow: var(--shadow-depth-8-focused);
outline: var(--outline-focus-visible);
outline-offset: 0.03125rem; // 0.5px
Comment on lines +18 to +19
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Use the shared outline-offset token here for consistency.

Line 19 hardcodes offset while the PR introduces --outline-focus-visible-offset. Reuse the token to keep focus visuals uniform across mixins.

♻️ Suggested patch 
-        outline-offset: 0.03125rem; // 0.5px
+        outline-offset: var(--outline-focus-visible-offset);
📝 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
outline: var(--outline-focus-visible);
outline-offset: 0.03125rem; // 0.5px
outline: var(--outline-focus-visible);
outline-offset: var(--outline-focus-visible-offset);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/style/mixins.scss` around lines 18 - 19, Replace the hardcoded
outline-offset in the mixin with the shared CSS variable by changing the
outline-offset: 0.03125rem line to use var(--outline-focus-visible-offset);
update the mixin in src/style/mixins.scss where outline:
var(--outline-focus-visible) is set so it references the new token
(--outline-focus-visible-offset) instead of the literal value, ensuring
consistency with other focus styles.

}
}

Expand Down Expand Up @@ -602,7 +602,9 @@ $clickable-normal-state-transitions: (
}

&:focus-visible {
box-shadow: var(--shadow-depth-8-focused), var(--button-shadow-hovered);
// box-shadow: var(--shadow-depth-8-focused), var(--button-shadow-hovered);
outline: var(--outline-focus-visible);
outline-offset: var(--outline-focus-visible-offset);
Comment on lines +605 to +607
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify mixin consumers and check for overflow clipping risk.
rg -nP '@include\s+the-3d-element--clickable\b' -g '**/*.scss' -C2
rg -nP 'overflow\s*:\s*hidden\s*;' -g '**/*.scss' -C2

Repository: Lundalogik/lime-elements

Length of output: 12528

🏁 Script executed:

# Search for the mixin definition
rg -n '@mixin the-3d-element--clickable' src/style/mixins.scss

# Check info-tile.scss to see how it actually uses styles
cat -n src/components/info-tile/info-tile.scss | head -100

# Search for the mixin in all files with variations
rg -n 'the-3d-element--clickable' -g '**/*.scss'

Repository: Lundalogik/lime-elements

Length of output: 4483

🏁 Script executed:

# Check the context around line 403 in info-tile.scss
sed -n '395,415p' src/components/info-tile/info-tile.scss

# View the mixin definition
sed -n '589,620p' src/style/mixins.scss

Repository: Lundalogik/lime-elements

Length of output: 1196

🏁 Script executed:

# Check for --shadow-focused-state variable
rg -n '--shadow-focused-state' -g '**/*.scss'

# Check for --shadow-depth-8-focused variable
rg -n '--shadow-depth-8-focused' -g '**/*.scss'

# Look for shadow variable definitions
rg -n 'shadow.*focused|focused.*shadow' -g '**/*.scss' | head -20

Repository: Lundalogik/lime-elements

Length of output: 2832

🏁 Script executed:

# Check other focus-visible implementations
rg -B2 -A2 '&:focus-visible' src/components -g '*.scss' | head -60

# Also check the shadows file to understand the variables better
sed -n '10,80p' src/style/shadows.scss

Repository: Lundalogik/lime-elements

Length of output: 7458

Outline-only focus indicator can be clipped in overflow: hidden consumers.

@mixin the-3d-element--clickable is used by components like src/components/info-tile/info-tile.scss (which sets overflow: hidden on the <a> element). With the focus ring moved to outline-only on lines 606–607, keyboard focus visibility can regress in those cases.

♻️ Suggested fix (restore box-shadow for focus state) 
     &:focus-visible {
-        outline: var(--outline-focus-visible);
-        outline-offset: var(--outline-focus-visible-offset);
+        box-shadow: var(--shadow-depth-8-focused), var(--button-shadow-hovered);
     }
📝 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
// box-shadow: var(--shadow-depth-8-focused), var(--button-shadow-hovered);
outline: var(--outline-focus-visible);
outline-offset: var(--outline-focus-visible-offset);
&:focus-visible {
box-shadow: var(--shadow-depth-8-focused), var(--button-shadow-hovered);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/style/mixins.scss` around lines 605 - 607, The focus indicator in the
`@mixin` the-3d-element--clickable currently uses outline-only (outline:
var(--outline-focus-visible); outline-offset:
var(--outline-focus-visible-offset);) which can be clipped by overflow: hidden
consumers; restore the box-shadow focus ring by reintroducing box-shadow:
var(--shadow-depth-8-focused), var(--button-shadow-hovered) alongside the
outline (for :focus and/or :focus-visible states) so keyboard focus remains
visible even when the element is clipped, ensuring the variables
--shadow-depth-8-focused and --button-shadow-hovered are used and kept in the
same focus rule as the outline.

}

&:focus-visible:active {
Expand Down
6 changes: 5 additions & 1 deletion src/style/shadows.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@
0 0.125rem 0.26rem rgb(var(--color-white), 0.06),
0 0 0 1px rgb(var(--color-white), 0.06);

--outline-focus-visible: 0.125rem solid
color-mix(in srgb, var(--mdc-theme-primary) 60%, transparent);
--outline-focus-visible-offset: 0.0125rem;

// Could be useful for highlighting areas or elements that are focused, using a box-shadow.
// However, we recommend to use `var(--shadow-depth-8-focus)` to get a more coherent visual effect.
--shadow-focused-state: 0 0 0 0.125rem
var(--lime-primary-color, var(--limel-theme-primary-color));
var(--lime-primary-color, var(--limel-theme-primary-color)) inset;

// Could be useful for highlighting areas or elements that contain errors, using a box-shadow.
// However, we recommend to use `var(--shadow-depth-8-error)` to get a more coherent visual effect.
Expand Down
Loading