Skip to content

Commit

Permalink
Merge pull request #5131 from nextcloud-libraries/fix/actions-focus-i…
Browse files Browse the repository at this point in the history
…n-avatar

fix(NcAvatar): Remove span wrapper button semantics in favour of internal button components
  • Loading branch information
Pytal authored Jan 26, 2024
2 parents 8e95526 + 16bff11 commit dceef1e
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 20 deletions.
3 changes: 0 additions & 3 deletions l10n/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,6 @@ msgstr ""
msgid "online"
msgstr ""

msgid "Open contact menu"
msgstr ""

msgid "Open link to \"{resourceName}\""
msgstr ""

Expand Down
13 changes: 13 additions & 0 deletions src/components/NcActions/NcActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,7 @@ export default {
'close',
'focus',
'blur',
'click',
],
data() {
Expand Down Expand Up @@ -1222,6 +1223,17 @@ export default {
onBlur(event) {
this.$emit('blur', event)
},
onClick(event) {
/**
* Event emitted when the menu toggle button is clicked.
*
* This is e.g. necessary for the NcAvatar component
* which needs to fetch the menu items on click.
*
* @type {PointerEvent}
*/
this.$emit('click', event)
},
},
/**
Expand Down Expand Up @@ -1420,6 +1432,7 @@ export default {
on: {
focus: this.onFocus,
blur: this.onBlur,
click: this.onClick,
},
}, [
h('template', { slot: 'icon' }, [triggerIcon]),
Expand Down
25 changes: 11 additions & 14 deletions src/components/NcAvatar/NcAvatar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,8 @@ export default {
'avatardiv--with-menu': hasMenu,
'avatardiv--with-menu-loading': contactsMenuLoading
}"
:title="tooltip"
:style="avatarStyle"
class="avatardiv popovermenu-wrapper"
:tabindex="hasMenu ? '0' : undefined"
:aria-label="avatarAriaLabel"
:role="hasMenu ? 'button' : undefined"
v-on="hasMenu ? {
click: toggleMenu,
keydown: toggleMenu,
} : null">
class="avatardiv popovermenu-wrapper">
<!-- @slot Icon slot -->
<slot name="icon">
<!-- Avatar icon or image -->
Expand All @@ -137,10 +129,12 @@ export default {

<!-- Contact menu -->
<!-- We show a button if the menu is not loaded yet. -->
<NcButton v-if="hasMenu && !menu.length"
:aria-label="t('Open contact menu')"
<NcButton v-if="hasMenu && menu.length === 0"
type="tertiary-no-background"
class="action-item action-item__menutoggle">
class="action-item action-item__menutoggle"
:aria-label="avatarAriaLabel"
:title="tooltip"
@click="toggleMenu">
<template #icon>
<NcLoadingIcon v-if="contactsMenuLoading" />
<DotsHorizontal v-else :size="20" />
Expand All @@ -151,7 +145,10 @@ export default {
manual-open
type="tertiary-no-background"
:container="menuContainer"
:open="contactsMenuOpenState">
:open.sync="contactsMenuOpenState"
:aria-label="avatarAriaLabel"
:title="tooltip"
@click="toggleMenu">
<NcActionLink v-for="(item, key) in menu"
:key="key"
:href="item.href"
Expand Down Expand Up @@ -747,7 +744,7 @@ export default {
cursor: pointer;
opacity: 0;
}
&:focus,
&:focus-within,
&:hover,
&#{&}-loading {
:deep(.action-item__menutoggle) {
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/components/NcAvatar/NcAvatar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('NcAvatar.vue', () => {
await nextTick()

expect(wrapper.find('.avatardiv__user-status').exists()).toBe(true)
expect(wrapper.attributes('aria-label')).toBe('Avatar of J. Doe, online')
expect(wrapper.find('.action-item__menutoggle').attributes('aria-label')).toBe('Avatar of J. Doe, online')
})

it('aria label is set to include status even if status is do-not-disturb', async () => {
Expand All @@ -63,7 +63,7 @@ describe('NcAvatar.vue', () => {
await nextTick()

expect(wrapper.find('.avatardiv__user-status').exists()).toBe(true)
expect(wrapper.attributes('aria-label')).toBe('Avatar of J. Doe, do not disturb')
expect(wrapper.find('.action-item__menutoggle').attributes('aria-label')).toBe('Avatar of J. Doe, do not disturb')
})

it('aria label is does not include status if status not shown', async () => {
Expand All @@ -85,7 +85,7 @@ describe('NcAvatar.vue', () => {
await nextTick()

expect(wrapper.find('.avatardiv__user-status').exists()).toBe(false)
expect(wrapper.attributes('aria-label')).toBe('Avatar of J. Doe')
expect(wrapper.find('.action-item__menutoggle').attributes('aria-label')).toBe('Avatar of J. Doe')
})

it('should display initials for user id', async () => {
Expand Down

0 comments on commit dceef1e

Please sign in to comment.