Skip to content

Commit 6e898f9

Browse files
pringelmannpringelmann
committed
fix(core): improve app icon contrast in the app menu
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
1 parent 8a0eaca commit 6e898f9

3 files changed

Lines changed: 124 additions & 32 deletions

File tree

‎apps/theming/tests/Themes/AccessibleThemeTestCase.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,16 @@ public static function dataAccessibilityPairs(): array {
8383
],
8484
$textContrast,
8585
],
86+
'primary-element on primary-element-light' => [
87+
[
88+
'--color-primary-element',
89+
],
90+
[
91+
'--color-primary-element-light',
92+
'--color-primary-element-light-hover',
93+
],
94+
$elementContrast,
95+
],
8696
'main-text' => [
8797
['--color-main-text'],
8898
[
@@ -230,4 +240,43 @@ public function testAccessibilityOfVariables(array $mainColors, array $backgroun
230240
}
231241
}
232242
}
243+
244+
/**
245+
* AppItem.vue gradients are not plain variables the pairs above can see, so
246+
* rebuild the lightest stop of each. Util::mix() weights its first colour at
247+
* ($factor + 100) / 200, which is how the CSS percentages map onto $factor.
248+
*/
249+
public function testAppMenuIconGradientContrast(): void {
250+
if (!isset($this->theme) || !isset($this->util)) {
251+
$this->markTestSkipped('You need to setup $this->theme and $this->util in your setUp function');
252+
}
253+
254+
$variables = $this->theme->getCSSVariables();
255+
$resolve = function (string $name) use ($variables): string {
256+
$matches = [];
257+
if (preg_match('/^var\\(([^)]+)\\)$/', $variables[$name], $matches) === 1) {
258+
return $variables[$matches[1]];
259+
}
260+
return $variables[$name];
261+
};
262+
263+
$primaryElement = $resolve('--color-primary-element');
264+
$circle = $resolve('--color-primary-element-light');
265+
$mainBackground = $resolve('--color-main-background');
266+
267+
// color-mix(in srgb, var(--color-primary-element), 28% var(--color-primary-element-light))
268+
$glyphTop = $this->util->mix($primaryElement, $circle, 44);
269+
// color-mix(in srgb, var(--color-primary-element-light), 15% var(--color-main-background))
270+
$circleTop = $this->util->mix($circle, $mainBackground, 70);
271+
272+
// The glyph is centred, so check it against both ends of the circle.
273+
foreach (['circle top' => $circleTop, 'circle bottom' => $circle] as $label => $background) {
274+
$contrast = $this->util->colorContrast($glyphTop, $background);
275+
$this->assertGreaterThanOrEqual(
276+
3.0,
277+
$contrast,
278+
"App menu glyph gradient top ($glyphTop) does not reach 3:1 on the $label ($background), got $contrast",
279+
);
280+
}
281+
}
233282
}

‎core/src/components/AppItem.vue‎

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@
1818
:title="app.name"
1919
role="menuitem">
2020
<span class="app-item__circle">
21-
<img
21+
<span
22+
v-if="app.icon"
2223
class="app-item__icon"
23-
:src="app.icon"
24-
alt=""
25-
aria-hidden="true">
24+
:style="iconStyle"
25+
aria-hidden="true" />
2626
<span
2727
v-if="app.unread"
2828
class="app-item__unread"
@@ -57,6 +57,11 @@ const props = withDefaults(defineProps<{
5757
tabindex: -1,
5858
})
5959
60+
// Escaped so a crafted path cannot break out of the url() token.
61+
const iconStyle = computed(() => ({
62+
'--app-item-icon-url': `url("${props.app.icon.replace(/["\\]/g, '\\$&')}")`,
63+
}))
64+
6065
const unreadLabel = computed(() => {
6166
if (!props.app.unread) {
6267
return undefined
@@ -72,63 +77,91 @@ const unreadLabel = computed(() => {
7277
</script>
7378

7479
<style scoped lang="scss">
80+
$bevel:
81+
inset 0 -1px 0 0 color-mix(in srgb, var(--color-primary-element-light), 10% var(--color-primary-element)),
82+
inset 0 -4px 6px -4px color-mix(in srgb, var(--color-primary-element-light), 16% var(--color-primary-element));
83+
7584
.app-item {
76-
--app-item-circle-size: calc(var(--default-grid-baseline) * 10);
77-
--app-item-icon-size: 22px;
85+
--app-item-circle-size: calc(var(--default-grid-baseline) * 12);
86+
// 28px on a 48px circle, so it follows when the circle is resized.
87+
--app-item-icon-size: calc(var(--app-item-circle-size) * 7 / 12);
88+
--app-item-bevel: #{$bevel};
7889
display: flex;
7990
flex-direction: column;
8091
align-items: center;
8192
gap: var(--default-grid-baseline);
82-
// Inset so the hover/focus highlight floats around the circle and label
83-
// rather than sitting flush against the icon at the top edge.
93+
// Keeps the grown circle and the focus ring off the tile's edge.
8494
padding-block: var(--default-grid-baseline);
8595
border-radius: var(--border-radius-element);
8696
text-decoration: none;
8797
color: var(--color-main-text);
8898
min-width: 0;
8999
90-
&:hover,
91-
&:focus-visible {
92-
background-color: var(--color-background-hover);
93-
}
94-
95100
// Inset ring instead of outline + offset: the offset version visibly
96101
// clips at the popover's rounded edge for items in the first/last row
97-
// or column. The inset shadow stays inside the highlight rectangle.
102+
// or column. The inset shadow stays inside the tile's own bounds.
98103
&:focus-visible {
99104
outline: none;
100105
box-shadow: inset 0 0 0 2px var(--color-primary-element);
101106
}
102107
108+
&:hover,
109+
&:focus-visible {
110+
--app-item-circle-scale: 1.08;
111+
}
112+
113+
&:active {
114+
--app-item-circle-scale: 0.96;
115+
}
116+
117+
@media (prefers-color-scheme: dark) {
118+
--app-item-bevel: none;
119+
}
120+
103121
&__circle {
104122
box-sizing: border-box;
105123
position: relative;
124+
display: flex;
125+
align-items: center;
126+
justify-content: center;
106127
width: var(--app-item-circle-size);
107128
height: var(--app-item-circle-size);
108129
border-radius: 50%;
109-
background-color: var(--color-primary-element);
130+
transform: scale(var(--app-item-circle-scale, 1));
131+
transition: transform var(--animation-quick) ease-out;
132+
background-color: var(--color-primary-element-light);
110133
background-image: linear-gradient(
111134
to bottom,
112-
rgba(255, 255, 255, 0.18) 0%,
113-
rgba(255, 255, 255, 0) 45%,
114-
rgba(0, 0, 0, 0.15) 100%
135+
color-mix(in srgb, var(--color-primary-element-light), 15% var(--color-main-background)) 0%,
136+
var(--color-primary-element-light) 100%
115137
);
116-
box-shadow:
117-
inset 0 1px 0 0 rgba(255, 255, 255, 0.25),
118-
inset 0 -1px 0 0 rgba(0, 0, 0, 0.2),
119-
0 2px 4px rgba(0, 0, 0, 0.15);
120-
display: flex;
121-
align-items: center;
122-
justify-content: center;
138+
box-shadow: var(--app-item-bevel);
139+
140+
@media (prefers-reduced-motion: reduce) {
141+
transition: none;
142+
}
123143
}
124144
125145
&__icon {
126146
width: var(--app-item-icon-size);
127147
height: var(--app-item-icon-size);
128-
// App icons are bright by default; flip them to dark when the
129-
// primary color (circle background) is bright (e.g. white in dark mode).
130-
filter: var(--primary-invert-if-bright);
131-
mask: var(--header-menu-icon-mask);
148+
// Masked rather than shown: app icons ship a hardcoded fill, so
149+
// currentColor never applies and a filter could only flip black and white.
150+
background-color: var(--color-primary-element);
151+
background-image: linear-gradient(
152+
to bottom,
153+
color-mix(in srgb, var(--color-primary-element), 28% var(--color-primary-element-light)) 0%,
154+
var(--color-primary-element) 100%
155+
);
156+
mask: var(--app-item-icon-url) center / contain no-repeat;
157+
}
158+
159+
// Masked backgrounds are not force-adjusted the way <img> is.
160+
@media (forced-colors: active) {
161+
&__icon {
162+
background-color: CanvasText;
163+
background-image: none;
164+
}
132165
}
133166
134167
&__unread {
@@ -169,8 +202,17 @@ const unreadLabel = computed(() => {
169202
}
170203
171204
&--outlined &__icon {
172-
filter: var(--background-invert-if-dark);
173-
mask: none;
205+
background-color: var(--color-main-text);
206+
background-image: none;
174207
}
175208
}
209+
210+
// An explicit theme choice must beat the media query above, which only sees the OS.
211+
:global([data-themes*=dark] .app-item) {
212+
--app-item-bevel: none;
213+
}
214+
215+
:global([data-themes*=light] .app-item) {
216+
--app-item-bevel: #{$bevel};
217+
}
176218
</style>

‎core/src/components/AppMenu.vue‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ export default defineComponent({
492492
493493
&__grid {
494494
--app-item-col-width: 69px;
495-
--app-item-row-height: 64px;
495+
--app-item-row-height: 72px;
496496
// border-box: the JS-set max-height (see recomputeGridMaxHeight)
497497
// needs to include padding for the peek math to hold.
498498
box-sizing: border-box;
@@ -502,6 +502,7 @@ export default defineComponent({
502502
grid-auto-rows: minmax(var(--app-item-row-height), max-content);
503503
// max-height set inline by recomputeGridMaxHeight(); CSS just owns the scroll.
504504
overflow-y: auto;
505+
overflow-x: hidden;
505506
506507
// Extra top padding on first-row tiles so the hover bg reads
507508
// concentric with the popover's rounded top corner. !important

0 commit comments

Comments
 (0)