Skip to content

fix: accessibility for apperance #62489

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
kristian-zendato wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: accessibility for apperance #62489

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
92 changes: 64 additions & 28 deletions apps/theming/src/components/ThemeListItem.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const props = defineProps<{
}>()

const imageUrl = computed(() => generateFilePath('theming', 'img', props.theme.id + '.jpg'))
const imageAlt = computed(() => t('theming', '{theme} preview', { theme: props.theme.title }))

const checkboxValue = computed({
get() {
if (props.type === 'switch') {
Expand All @@ -55,37 +57,44 @@ const checkboxValue = computed({
isSelected.value = !isSelected.value
},
})

</script>

<template>
<li :class="'theming__preview--' + theme.id" class="theming__preview">
<img
alt=""
class="theming__preview-image"
:src="imageUrl"
<button

@susnux susnux Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not valid you cannot have a checkbox inside a button.
This should be just a DIV element and checkbox within already is the semantic input element.

@susnux susnux Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So the outer elements needs to be a div and the outer fieldset needs a role="radiogroup".

type="button"
class="theming__preview-option"
:disabled="props.enforced || props.loading"
@click="isSelected = !isSelected">
<div class="theming__preview-description">
<h3>{{ theme.title }}</h3>
<p class="theming__preview-explanation">
{{ theme.description }}
</p>
<span v-if="enforced" class="theming__preview-warning" role="note">
{{ t('theming', 'Theme selection is enforced') }}
</span>

<!-- Only show checkbox if we can change themes -->
<NcCheckboxRadioSwitch
v-show="!enforced"
v-model="checkboxValue"
class="theming__preview-toggle"
:disabled="enforced"
:loading
:name="type !== 'switch' ? name : undefined"
:type
:value="type !== 'switch' ? theme.id : undefined">
{{ theme.enableLabel }}
</NcCheckboxRadioSwitch>
</div>
<img
class="theming__preview-image"
:alt="imageAlt"
:src="imageUrl">
<div class="theming__preview-description">
<h3>{{ theme.title }}</h3>
<p class="theming__preview-explanation">
{{ theme.description }}
</p>
<span v-if="enforced" class="theming__preview-warning" role="note">
{{ t('theming', 'Theme selection is enforced') }}
</span>

<!-- Only show checkbox if we can change themes -->
<NcCheckboxRadioSwitch
v-show="!enforced"
v-model="checkboxValue"
tabindex="-1"
class="theming__preview-toggle"
:disabled="enforced"
:loading
:name="type !== 'switch' ? name : undefined"
:type
:value="type !== 'switch' ? theme.id : undefined">
{{ theme.enableLabel }}
</NcCheckboxRadioSwitch>
</div>
</button>
</li>
</template>

Expand All @@ -104,16 +113,37 @@ const checkboxValue = computed({
user-select: none;
}

&-option {
display: flex;
justify-content: flex-start;
width: 100%;
margin: 0;
padding: 0;
cursor: pointer;
border: none;
border-radius: var(--border-radius);
background: none;
text-align: start;

&:disabled {
background-color: transparent;
}

&:hover:not(:disabled) {
background-color: color-mix(in srgb, var(--color-primary-element) 8%, transparent);
}
}

&-image {
flex-basis: calc(16px * var(--ratio));
flex-shrink: 0;
height: calc(10px * var(--ratio));
margin-inline-end: var(--gap);
cursor: pointer;
border-radius: var(--border-radius);
background-repeat: no-repeat;
background-position: top left;
background-size: cover;
pointer-events: none;
}

&-explanation {
Expand All @@ -134,6 +164,10 @@ const checkboxValue = computed({
}
}

&-toggle {
pointer-events: none;
}

&-warning {
background-color: var(--color-warning);
color: var(--color-warning-text);
Expand All @@ -142,7 +176,9 @@ const checkboxValue = computed({

@media (max-width: math.div(1024px, 1.5)) {
.theming__preview {
flex-direction: column;
&-option {
flex-direction: column;
}

&-image {
margin: 0;
Expand Down
64 changes: 44 additions & 20 deletions apps/theming/src/components/UserSectionBackground.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,11 @@ async function pickFile() {
class="background"
:name="t('theming', 'Background and color')"
:description="t('theming', 'The background can be set to an image from the default set, a custom uploaded image, or a plain color.')">
<fieldset>
<legend class="hidden-visually">
{{ t('theming', 'Background and color') }}
</legend>

<div :class="$style.backgroundSelect">
<!-- Custom background -->
<ul

@susnux susnux Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why?
This is semantically not really correct, fieldset already was correct.
As this is a group of related input fileds. Its not really a list.

@kristian-zendato kristian-zendato Jul 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

https://github.com/nextcloud-gmbh/customer-feature-requests/issues/1624

Related to this feedback, they want the background to be HTML list.

@susnux susnux Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But thats invalid see my feedback on the issue. IMHO a false-positive text and should be closed as wont fix with proper reference to WCAG rules as the current semantics are correct (expect my other comment above).

:aria-label="t('theming', 'Background and color')"
:class="$style.backgroundSelect">
<!-- Custom background -->
<li :class="$style.backgroundSelect__item" role="listitem">
<button
:aria-disabled="loading === 'custom'"
:aria-pressed="currentBackgroundImage === 'custom'"
Expand All @@ -184,8 +182,10 @@ async function pickFile() {
<NcLoadingIcon v-if="loading === 'custom'" />
<NcIconSvgWrapper v-else :path="currentBackgroundImage === 'custom' ? mdiCheck : mdiImageEditOutline" />
</button>
</li>

<!-- Custom color picker -->
<!-- Custom color picker -->
<li :class="$style.backgroundSelect__item" role="listitem">
<NcColorPicker v-model="currentTheming.backgroundColor" @submit="pickColor">
<button
class="button-vue"
Expand All @@ -202,8 +202,10 @@ async function pickFile() {
<NcIconSvgWrapper v-else :path="currentBackgroundImage === 'color' ? mdiCheck : mdiPaletteOutline" />
</button>
</NcColorPicker>
</li>

<!-- Default background -->
<!-- Default background -->
<li :class="$style.backgroundSelect__item" role="listitem">
<button
class="button-vue"
:class="[$style.backgroundSelect__entry, $style.backgroundSelect__entryDefault]"
Expand All @@ -218,24 +220,28 @@ async function pickFile() {
<NcLoadingIcon v-if="loading === 'default'" />
<NcIconSvgWrapper v-else :path="currentBackgroundImage === 'default' ? mdiCheck : mdiUndo" />
</button>
</div>
</li>

<!-- Force shipped backgrounds onto the next line while keeping flex wrap layout -->
<li
:class="$style.backgroundSelect__break"
aria-hidden="true"
role="presentation" />

<!-- Background set selection -->
<fieldset :class="$style.backgroundSelect">
<label class="hidden-visually">
{{ t('theming', 'Default shipped background images') }}
</label>
<!-- Shipped background set -->
<li
v-for="shippedBackground in shippedBackgrounds"
:key="shippedBackground.name"
:class="$style.backgroundSelect__item"
role="listitem">
<button
v-for="shippedBackground in shippedBackgrounds"
:key="shippedBackground.name"
:title="shippedBackground.details.attribution"
:aria-pressed="currentBackgroundImage === shippedBackground.name"
class="button-vue"
:class="$style.backgroundSelect__entry"
:style="{
backgroundImage: 'url(' + shippedBackground.preview + ')',
}"
tabindex="0"
@click="setShipped(shippedBackground.name)">
<NcIconSvgWrapper
v-if="currentBackgroundImage === shippedBackground.name"
Expand All @@ -245,8 +251,8 @@ async function pickFile() {
{{ shippedBackground.details.description }}
</span>
</button>
</fieldset>
</fieldset>
</li>
</ul>
</NcSettingsSection>
</template>

Expand All @@ -255,6 +261,24 @@ async function pickFile() {
display: flex;
flex-wrap: wrap;
justify-content: center;
margin: 0;
padding: 0;
list-style: none;

.backgroundSelect__item {
display: flex;
list-style: none;
}

.backgroundSelect__break {
flex-basis: 100%;
width: 100%;
height: 0;
margin: 0;
padding: 0;
overflow: hidden;
list-style: none;
}

.backgroundSelect__entry {
display: flex;
Expand Down
2 changes: 1 addition & 1 deletion dist/theming-settings-personal.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* extracted by css-entry-points-plugin */
@import './theming-theming-settings-personal-Bq6hfFAy.chunk.css';
@import './theming-theming-settings-personal-B8jY0rO7.chunk.css';
@import './common-createElementId-DhjFt1I9-C_oBIsvc.chunk.css';
@import './common-TrashCanOutline-BYHcrfvW.chunk.css';
@import './common-NcCheckboxRadioSwitch-DVdt5Hkq-Ct9ZGMmH.chunk.css';
Expand Down
2 changes: 1 addition & 1 deletion dist/theming-settings-personal.mjs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/theming-settings-personal.mjs.map
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions dist/theming-theming-settings-personal-B8jY0rO7.chunk.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion dist/theming-theming-settings-personal-Bq6hfFAy.chunk.css
View file
Open in desktop

This file was deleted.

Loading