Skip to content
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

Jump to bottom

A few accessibility tweaks #4652

Merged
merged 12 commits into from
Nov 25, 2020
Merged

A few accessibility tweaks #4652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ae28569
Define region roles for easier access
PVince81 Nov 19, 2020
83e1820
Add missing or clarify aria labels in lists
PVince81 Nov 19, 2020
20cabbf
Hide redundant elements from screen readers
PVince81 Nov 19, 2020
5881be7
Message groups are now lists
PVince81 Nov 19, 2020
ba1d389
Give heading role to message group author and day separator
PVince81 Nov 19, 2020
4c66f59
Caption is actually a heading
PVince81 Nov 19, 2020
bdf9d0c
Allow tabbing into participant search results
PVince81 Nov 19, 2020
bf4c9a2
Add missing aria label on new message form
PVince81 Nov 19, 2020
c6a845b
Adjust headings in messages list to be more coherent
PVince81 Nov 23, 2020
e067f64
Adjusted heading levels in messages list
PVince81 Nov 23, 2020
773be42
Mark more li items as presentation in left sidebar
PVince81 Nov 23, 2020
063fe16
Only set aria label to match nextcloud-vue definition
PVince81 Nov 23, 2020
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
2 changes: 1 addition & 1 deletion src/components/Caption.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
-->

<template>
<li class="app-navigation-caption">
<li role="heading" class="app-navigation-caption">
{{ title }}
</li>
</template>
Expand Down
6 changes: 5 additions & 1 deletion src/components/ChatView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,12 @@
</div>
</transition>
<MessagesList
role="region"
:aria-label="t('spreed', 'Conversation messages')"
:token="token" />
<NewMessageForm />
<NewMessageForm
role="region"
:aria-label="t('spreed', 'Post message')" />
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this go within the newmessageform component?

Copy link
Member Author

Choose a reason for hiding this comment

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

not this one: see I've defined a "role=region" which means that screen readers will announce this section as a region.
with a screen reader you have the option to jump directly to a specific region of the screen from anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

seems I had a separate PR for the other missing aria-label, see #4648

Copy link
Member

Choose a reason for hiding this comment

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

Just sayinh that maybe those 2 lines should go here?

spreed/src/components/NewMessageForm/NewMessageForm.vue

Line 24 in 1bce4b8

class="wrapper">

Aren't they going to end up on the newmessageform root element anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes and no

We need to keep the aria-label here so that the region itself has a name. The screen reader will announce it "Entering post message region" when tabbing into it, even if you tab onto the file upload button first so we should not set the region on the post message field.

I agree that the field itself also needs an aria-label.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've cherry-picked my changes from the other PR here to make it more complete.

Please let me know whether you understood the region thing. I can demo it to you if you like.

</div>
</template>

Expand Down
11 changes: 9 additions & 2 deletions src/components/LeftSidebar/ConversationsList/AppContentListItem/AppContentListItem.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
:class="{ 'active' : isActive }"
href="#"
class="acli"
:aria-label="t('spreed', 'Conversation, ') + title"
:aria-label="conversationLinkAriaLabel"
@click="onClick">
<!-- default slot for avatar or icon -->
<slot name="icon" />
Expand Down Expand Up @@ -57,7 +57,7 @@
<Actions
v-if="hasActions"
menu-align="right"
:aria-label="t('spreed', 'Conversation settings')"
:aria-label="conversationSettingsAriaLabel"
class="actions">
<slot name="actions" />
</Actions>
Expand Down Expand Up @@ -138,6 +138,13 @@ export default {
}
},

conversationLinkAriaLabel() {
return t('spreed', 'Conversation "{conversationName}"', { conversationName: this.title })
},

conversationSettingsAriaLabel() {
return t('spreed', 'Settings for conversation "{conversationName}"', { conversationName: this.title })
},
},
methods: {
// forward click event
Expand Down
10 changes: 5 additions & 5 deletions src/components/LeftSidebar/LeftSidebar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
-->

<template>
<AppNavigation>
<AppNavigation :aria-label="t('spreed', 'Conversation list')">
<div class="new-conversation">
<SearchBox
v-model="searchText"
Expand All @@ -35,7 +35,7 @@
<template #list class="left-sidebar__list">
<Caption v-if="isSearching"
:title="t('spreed', 'Conversations')" />
<li>
<li role="presentation">
<ConversationsList
:conversations-list="conversationsList"
:initialised-conversations="initialisedConversations"
Expand All @@ -47,7 +47,7 @@
<template v-if="searchResultsUsers.length !== 0">
<Caption
:title="t('spreed', 'Users')" />
<li v-if="searchResultsUsers.length !== 0">
<li v-if="searchResultsUsers.length !== 0" role="presentation">
<ConversationsOptionsList
:items="searchResultsUsers"
@click="createAndJoinConversation" />
Expand All @@ -64,7 +64,7 @@
<template v-if="searchResultsGroups.length !== 0">
<Caption
:title="t('spreed', 'Groups')" />
<li v-if="searchResultsGroups.length !== 0">
<li v-if="searchResultsGroups.length !== 0" role="presentation">
<ConversationsOptionsList
:items="searchResultsGroups"
@click="createAndJoinConversation" />
Expand All @@ -74,7 +74,7 @@
<template v-if="searchResultsCircles.length !== 0">
<Caption
:title="t('spreed', 'Circles')" />
<li v-if="searchResultsCircles.length !== 0">
<li v-if="searchResultsCircles.length !== 0" role="presentation">
<ConversationsOptionsList
:items="searchResultsCircles"
@click="createAndJoinConversation" />
Expand Down
16 changes: 8 additions & 8 deletions src/components/MessagesList/MessagesGroup/Message/Message.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ the main body of the message as well as a quote.
</docs>

<template>
<div
<li
marcoambrosini marked this conversation as resolved.
Show resolved Hide resolved
:id="`message_${id}`"
ref="message"
class="message"
:class="{'hover': showActions && !isSystemMessage, 'system' : isSystemMessage}"
@mouseover="showActions=true"
@mouseleave="showActions=false">
<div v-if="isFirstMessage && showAuthor" class="message__author">
<h6>{{ actorDisplayName }}</h6>
<div v-if="isFirstMessage && showAuthor"
class="message__author"
role="heading"
aria-level="4">
{{ actorDisplayName }}
</div>
<div
ref="messageMain"
Expand All @@ -55,10 +58,7 @@ the main body of the message as well as a quote.
</div>
<div class="message__main__right">
<div v-if="isTemporary && !isTemporaryUpload" class="icon-loading-small" />
<h6 v-if="hasDate"
v-tooltip.auto="messageDate">
{{ messageTime }}
</h6>
<span v-if="hasDate" v-tooltip.auto="messageDate">{{ messageTime }}</span>
<Actions
v-show="showActions && hasActions"
class="message__main__right__actions">
Expand All @@ -72,7 +72,7 @@ the main body of the message as well as a quote.
</Actions>
</div>
</div>
</div>
</li>
</template>

<script>
Expand Down
6 changes: 3 additions & 3 deletions src/components/MessagesList/MessagesGroup/MessagesGroup.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<template>
<div class="message-group">
<div v-if="dateSeparator" class="message-group__date-header">
<span class="date">{{ dateSeparator }}</span>
<span class="date" role="heading" aria-level="3">{{ dateSeparator }}</span>
</div>
<div class="wrapper">
<div class="messages__avatar">
Expand All @@ -31,7 +31,7 @@
:author-id="actorId"
:display-name="actorDisplayName" />
</div>
<div class="messages">
<ul class="messages">
Copy link
Member

Choose a reason for hiding this comment

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

The list is ol though, as it’s ordered, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

indeed, had the same thought.
technically doesn't make a difference for assistive technologies from what I read

Copy link
Member

@marcoambrosini marcoambrosini Nov 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The only difference afaik is visual in default styles. Bullets in ul and umbers in ol

Copy link
Member

Choose a reason for hiding this comment

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

Also when scrolling up new items would be added and therefor all numbers would change. sounds confusing?

<Message
v-for="(message, index) of messages"
:key="message.id"
Expand All @@ -42,7 +42,7 @@
:actor-display-name="actorDisplayName"
:show-author="!isSystemMessage"
:is-temporary="message.timestamp === 0" />
</div>
</ul>
</div>
</div>
</template>
Expand Down
6 changes: 6 additions & 0 deletions src/components/NewMessageForm/AdvancedInput/AdvancedInput.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
:contenteditable="activeInput"
:placeHolder="placeholderText"
role="textbox"
:aria-label="ariaLabel"
aria-multiline="true"
class="new-message-form__advancedinput"
@shortkey="focusInput"
Expand Down Expand Up @@ -178,6 +179,11 @@ export default {
default: '',
},

ariaLabel: {
type: String,
default: '',
},

/**
* Determines if the input is active
*/
Expand Down
3 changes: 3 additions & 0 deletions src/components/NewMessageForm/NewMessageForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
ref="fileUploadInput"
multiple
type="file"
tabindex="-1"
aria-hidden="true"
class="hidden-visually"
@change="handleFileInput">
<div
Expand Down Expand Up @@ -84,6 +86,7 @@
:token="token"
:active-input="!isReadOnly"
:placeholder-text="placeholderText"
:aria-label="placeholderText"
@update:contentEditable="contentEditableToParsed"
@submit="handleSubmit"
@files-pasted="handlePastedFiles" />
Expand Down
4 changes: 2 additions & 2 deletions src/components/Quote.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ components.
class="quote"
@click.prevent="handleQuoteClick">
<div class="quote__main">
<div class="quote__main__author">
<h6>{{ getDisplayName }}</h6>
<div class="quote__main__author" role="heading" aria-level="4">
{{ getDisplayName }}
</div>
<div v-if="isFileShareMessage"
class="quote__main__text">
Expand Down
24 changes: 22 additions & 2 deletions src/components/RightSidebar/Participants/ParticipantsList/Participant/Participant.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@
'guestUser': isGuest,
'isSearched': isSearched,
'selected': isSelected }"
@click="handleClick">
:aria-label="participantAriaLabel"
:role="isSearched ? 'listitem' : ''"
:tabindex="isSearched ? 0 : -1"
v-on="isSearched ? { click: handleClick, 'keydown.enter': handleClick } : {}"
@keydown.enter="handleClick">
<AvatarWrapper
:id="computedId"
:disable-tooltip="true"
:disable-menu="isSearched"
:size="44"
:show-user-status="showUserStatus && !isSearched"
:show-user-status-compact="false"
Expand Down Expand Up @@ -79,7 +84,7 @@
</div>
<Actions
v-if="canModerate && !isSearched"
:aria-label="t('spreed', 'Participant settings')"
:aria-label="participantSettingsAriaLabel"
class="participant-row__actions">
<ActionButton v-if="canBeDemoted"
icon="icon-rename"
Expand Down Expand Up @@ -158,6 +163,17 @@ export default {
},

computed: {
participantSettingsAriaLabel() {
return t('spreed', 'Settings for participant "{user}"', { user: this.computedName })
},
participantAriaLabel() {
if (this.isSearched) {
return t('spreed', 'Add participant "{user}"', { user: this.computedName })
} else {
return t('spreed', 'Participant "{user}"', { user: this.computedName })
}
},

userTooltipText() {
if (!this.isUserNameTooltipVisible) {
return false
Expand Down Expand Up @@ -376,6 +392,10 @@ export default {
cursor: pointer;
}

&:focus {
background-color: var(--color-background-hover);
}

&__user-wrapper {
margin-top: -4px;
margin-left: 12px;
Expand Down
1 change: 1 addition & 0 deletions src/components/TopBar/TopBar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
v-shortkey="['f']"
class="top-bar__button"
menu-align="right"
:aria-label="t('spreed', 'Conversation actions')"
@shortkey.native="toggleFullscreen">
<ActionButton
:icon="iconFullscreen"
Expand Down