Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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 l10n/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ msgstr ""
msgid "Open menu"
msgstr ""

msgid "Open navigation"
msgid "Open navigation {shortcut}"
msgstr ""

msgid "Open sidebar"
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"splitpanes": "^3.1.8",
"string-length": "^6.0.0",
"striptags": "^3.2.0",
"tabbable": "^6.2.0",
"tributejs": "^5.1.3",
"unified": "^11.0.5",
"unist-builder": "^4.0.0",
Expand Down
54 changes: 49 additions & 5 deletions src/components/NcAppNavigation/NcAppNavigation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,13 @@ emit('toggle-navigation', {
</template>

<script>
import { useIsMobile } from '../../composables/useIsMobile/index.js'
import { getTrapStack } from '../../utils/focusTrap.js'
import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
import { createFocusTrap } from 'focus-trap'
import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
import { tabbable } from 'tabbable'

import { getTrapStack } from '../../utils/focusTrap.js'
import { useHotKey } from '../../composables/useHotKey/index.js'
import { useIsMobile } from '../../composables/useIsMobile/index.js'

import NcAppNavigationToggle from '../NcAppNavigationToggle/index.js'
import NcAppNavigationList from '../NcAppNavigationList/index.js'
Expand Down Expand Up @@ -246,6 +249,12 @@ export default {
escapeDeactivates: false,
})
this.toggleFocusTrap()

// N opens + focuses the navigation
useHotKey('n', this.onKeyDown, {
prevent: true,
stop: true,
})
},
unmounted() {
this.setHasAppNavigation(false)
Expand All @@ -259,7 +268,7 @@ export default {
*
* @param {boolean} [state] set the state instead of inverting the current one
*/
toggleNavigation(state) {
async toggleNavigation(state) {
// Early return if already in that state
if (this.open === state) {
emit('navigation-toggled', {
Expand All @@ -272,6 +281,12 @@ export default {
const bodyStyles = getComputedStyle(document.body)
const animationLength = parseInt(bodyStyles.getPropertyValue('--animation-quick')) || 100

// If we just opened, we focus the first element
if (this.open) {
await this.$nextTick()
this.focusFirstElement()
}

setTimeout(() => {
emit('navigation-toggled', {
open: this.open,
Expand All @@ -296,10 +311,39 @@ export default {
},

handleEsc() {
if (this.isMobile) {
if (this.isMobile && this.open) {
this.toggleNavigation(false)
}
},

focusFirstElement() {
const element = tabbable(this.$refs.appNavigationContainer)[0]
if (element) {
element.focus()
logger.debug('Focusing first element in the navigation', { element })
}
},

onKeyDown(event) {
// toggle the navigation on 'n' key
if (event.key === 'n') {
// If the navigation is closed, open it
if (!this.open) {
this.toggleNavigation(true)
return
}

// If the navigation is open and the focus is within the navigation, close it
if (this.isFocusWithinNavigation()) {
this.toggleNavigation(false)
}
}
},

isFocusWithinNavigation() {
const activeElement = document.activeElement
return this.$refs.appNavigationContainer.contains(activeElement)
},
},
}
</script>
Expand Down
11 changes: 10 additions & 1 deletion src/components/NcAppNavigationToggle/NcAppNavigationToggle.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
:aria-label="label"
:title="label"
aria-controls="app-navigation-vue"
:aria-keyshortcuts="disableKeyboardShortcuts ? '' : 'n'"
@click="toggleNavigation">
<template #icon>
<MenuOpenIcon v-if="open" :size="20" />
Expand All @@ -27,6 +28,8 @@ import { t } from '../../l10n.js'
import MenuIcon from 'vue-material-design-icons/Menu.vue'
import MenuOpenIcon from 'vue-material-design-icons/MenuOpen.vue'

const disableKeyboardShortcuts = window.OCP?.Accessibility?.disableKeyboardShortcuts?.()

export default {
name: 'NcAppNavigationToggle',

Expand All @@ -50,9 +53,15 @@ export default {

emits: ['update:open'],

setup() {
return { disableKeyboardShortcuts }
},

computed: {
label() {
return this.open ? t('Close navigation') : t('Open navigation')
return this.open
? t('Close navigation')
: t('Open navigation {shortcut}', { shortcut: disableKeyboardShortcuts ? '' : '[n]' }).trim()
},
},
methods: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/NcPasswordField/NcPasswordField.vue
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ import axios from '@nextcloud/axios'
import { loadState } from '@nextcloud/initial-state'
import { generateOcsUrl } from '@nextcloud/router'
import { t } from '../../l10n.js'
import logger from '../../utils/logger.js'
import { logger } from '../../utils/logger.ts'

/**
* @typedef PasswordPolicy
Expand Down
2 changes: 1 addition & 1 deletion src/utils/logger.js → src/utils/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { getLoggerBuilder } from '@nextcloud/logger'

export default getLoggerBuilder()
export const logger = getLoggerBuilder()
.detectUser()
.setApp('@nextcloud/vue')
.build()
81 changes: 81 additions & 0 deletions tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { HooksConfig } from '../../setup/index'
import { expect, test } from '@playwright/experimental-ct-vue'

import AppNavigation from './NcAppNavigation.story.vue'

test.skip(({ browserName }) => browserName !== 'chromium')

// A little bit hacky but we test a wrapper element so we need to use the real NcContent and NcAppNavigation
test.beforeEach(async ({ mount, page }) => {
const handle = await page.locator('#app-content').elementHandle()
expect(handle).not.toBeNull()
await handle!.evaluate((node) => { node.innerHTML = ''; node.id = 'root' })

await mount<HooksConfig>(AppNavigation, {
hooksConfig: {
routes: [
{ path: '/', component: AppNavigation },
{ path: '/foo', component: AppNavigation },
],
},
})
})

test('opens on n keyboard press', async ({ page }) => {
await expect(page.getByText('First')).toBeVisible();

// cy.get('nav').then(($nav) => {
// const id = $nav.attr('id')
// cy.get(`[aria-controls="${id}"`).as('appNavigationToggle')
// cy.get('@appNavigationToggle').should('have.attr', 'aria-expanded', 'true')
// cy.get('nav').should('have.attr', 'aria-hidden', 'false')
// cy.get('nav').should('not.have.attr', 'inert')

// // close the sidebar
// cy.get('@appNavigationToggle').click()

// cy.get('@appNavigationToggle').should('have.attr', 'aria-expanded', 'false')
// cy.get('nav').should('have.attr', 'aria-hidden', 'true')
// cy.get('nav').should('have.attr', 'inert')

// // open the sidebar with the keyboard
// cy.get('body').type('n')

// cy.get('@appNavigationToggle').should('have.attr', 'aria-expanded', 'true')
// cy.get('nav').should('have.attr', 'aria-hidden', 'false')
// cy.get('nav').should('not.have.attr', 'inert')

// // make sure we auto-focus the first item
// cy.document().then((doc) => {
// const activeElement = doc.activeElement
// const navigation = doc.querySelector('nav')
// // eslint-disable-next-line no-unused-expressions
// expect(navigation?.contains(activeElement)).to.be.true
// })
// })
})

test('closes on n keyboard press', async ({ page }) => {
await expect(page.getByText('First')).toBeVisible()
const navigation = page.getByRole('navigation')

await expect(navigation).toHaveAttribute('aria-hidden', 'false')
await expect(navigation).not.toHaveAttribute('inert')

// pressing n does nothing until we focus something within
page.locator('body').press('n')
await expect(navigation).toHaveAttribute('aria-hidden', 'false')
await expect(navigation).not.toHaveAttribute('inert')

// focus something within
navigation.getByRole('link').first().focus()

// pressing n closes the sidebar
page.locator('body').press('n')
await expect(navigation).toHaveAttribute('aria-hidden', 'true')

Check failure on line 79 in tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigation/NcAppNavigation.spec.ts:62:1 › closes on n keyboard press

1) [chromium] › components/NcAppNavigation/NcAppNavigation.spec.ts:62:1 › closes on n keyboard press Error: Timed out 5000ms waiting for expect(locator).toHaveAttribute(expected) Locator: getByRole('navigation') Expected string: "true" Received: <element(s) not found> Call log: - expect.toHaveAttribute with timeout 5000ms - waiting for getByRole('navigation') 2 × locator resolved to <nav data-v-513948c6="" aria-hidden="false" id="app-navigation-vue" aria-label="In-app navigation" class="app-navigation__content">…</nav> - unexpected value "false" 77 | // pressing n closes the sidebar 78 | page.locator('body').press('n') > 79 | await expect(navigation).toHaveAttribute('aria-hidden', 'true') | ^ 80 | await expect(navigation).toHaveAttribute('inert') 81 | }) 82 | at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts:79:27

Check failure on line 79 in tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigation/NcAppNavigation.spec.ts:62:1 › closes on n keyboard press

1) [chromium] › components/NcAppNavigation/NcAppNavigation.spec.ts:62:1 › closes on n keyboard press Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(locator).toHaveAttribute(expected) Locator: getByRole('navigation') Expected string: "true" Received: <element(s) not found> Call log: - expect.toHaveAttribute with timeout 5000ms - waiting for getByRole('navigation') 2 × locator resolved to <nav data-v-513948c6="" aria-hidden="false" id="app-navigation-vue" aria-label="In-app navigation" class="app-navigation__content">…</nav> - unexpected value "false" 77 | // pressing n closes the sidebar 78 | page.locator('body').press('n') > 79 | await expect(navigation).toHaveAttribute('aria-hidden', 'true') | ^ 80 | await expect(navigation).toHaveAttribute('inert') 81 | }) 82 | at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts:79:27

Check failure on line 79 in tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigation/NcAppNavigation.spec.ts:62:1 › closes on n keyboard press

1) [chromium] › components/NcAppNavigation/NcAppNavigation.spec.ts:62:1 › closes on n keyboard press Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(locator).toHaveAttribute(expected) Locator: getByRole('navigation') Expected string: "true" Received: <element(s) not found> Call log: - expect.toHaveAttribute with timeout 5000ms - waiting for getByRole('navigation') 2 × locator resolved to <nav data-v-513948c6="" aria-hidden="false" id="app-navigation-vue" aria-label="In-app navigation" class="app-navigation__content">…</nav> - unexpected value "false" 77 | // pressing n closes the sidebar 78 | page.locator('body').press('n') > 79 | await expect(navigation).toHaveAttribute('aria-hidden', 'true') | ^ 80 | await expect(navigation).toHaveAttribute('inert') 81 | }) 82 | at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigation/NcAppNavigation.spec.ts:79:27
await expect(navigation).toHaveAttribute('inert')
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!--
- SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: AGPL-3.0-or-later
-->

<template>
<NcContent app-name="testing">
<NcAppNavigation aria-label="In-app navigation">
<template #list>
<NcAppNavigationItem name="First" />
</template>
</NcAppNavigation>
<NcAppContent />
</NcContent>
</template>

<script setup lang="ts">
import NcAppContent from '../../../../src/components/NcAppContent/NcAppContent.vue'
import NcAppNavigation from '../../../../src/components/NcAppNavigation/NcAppNavigation.vue'
import NcAppNavigationItem from '../../../../src/components/NcAppNavigationItem/NcAppNavigationItem.vue'
import NcContent from '../../../../src/components/NcContent/NcContent.vue'
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import type { HooksConfig } from '../../setup/index'
import { expect, test } from '@playwright/experimental-ct-vue'

import AppNavigation from './AppNavigation.story.vue'
import AppNavigation from './NcAppNavigationItem.story.vue'

test.skip(({ browserName }) => browserName !== 'chromium')

Expand All @@ -29,7 +29,7 @@
const navigation = page.getByRole('navigation', { name: 'In-app navigation' })

await expect(navigation).toBeVisible()
await expect(navigation.getByRole('listitem').filter({ hasText: 'Home' })).toHaveScreenshot()

Check failure on line 32 in tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:28:1 › has primary styling on active route @visual

2) [chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:28:1 › has primary styling on active route @visual Error: A snapshot doesn't exist at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/snapshots/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts-snapshots/has-primary-styling-on-active-route-1-chromium-linux.png, writing actual. 30 | 31 | await expect(navigation).toBeVisible() > 32 | await expect(navigation.getByRole('listitem').filter({ hasText: 'Home' })).toHaveScreenshot() | ^ 33 | }) 34 | 35 | test('has primary button styling on active route with editing=true', { tag: '@visual' }, async ({ page }) => { at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:32:2
})

test('has primary button styling on active route with editing=true', { tag: '@visual' }, async ({ page }) => {
Expand All @@ -42,7 +42,7 @@
await item.getByRole('button', { name: 'Edit item' }).click()
await expect(item.getByRole('textbox')).toBeVisible()

await expect(item).toHaveScreenshot()

Check failure on line 45 in tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:35:1 › has primary button styling on active route with editing=true @visual

3) [chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:35:1 › has primary button styling on active route with editing=true @visual Error: A snapshot doesn't exist at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/snapshots/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts-snapshots/has-primary-button-styling-on-active-route-with-editing-true-1-chromium-linux.png, writing actual. 43 | await expect(item.getByRole('textbox')).toBeVisible() 44 | > 45 | await expect(item).toHaveScreenshot() | ^ 46 | }) 47 | 48 | test('has tertiary styling on non active route', { tag: '@visual' }, async ({ page }) => { at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:45:2
})

test('has tertiary styling on non active route', { tag: '@visual' }, async ({ page }) => {
Expand All @@ -50,7 +50,7 @@
const item = navigation.getByRole('listitem').filter({ hasText: 'Foo' })

await expect(navigation).toBeVisible()
await expect(item).toHaveScreenshot()

Check failure on line 53 in tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:48:1 › has tertiary styling on non active route @visual

4) [chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:48:1 › has tertiary styling on non active route @visual Error: A snapshot doesn't exist at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/snapshots/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts-snapshots/has-tertiary-styling-on-non-active-route-1-chromium-linux.png, writing actual. 51 | 52 | await expect(navigation).toBeVisible() > 53 | await expect(item).toHaveScreenshot() | ^ 54 | }) 55 | 56 | test('has primary styling on active entry', { tag: '@visual' }, async ({ page }) => { at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:53:2
})

test('has primary styling on active entry', { tag: '@visual' }, async ({ page }) => {
Expand All @@ -58,7 +58,7 @@
const item = navigation.getByRole('listitem').filter({ hasText: 'Back' })

await expect(navigation).toBeVisible()
await expect(item).toHaveScreenshot()

Check failure on line 61 in tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:56:1 › has primary styling on active entry @visual

5) [chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:56:1 › has primary styling on active entry @visual Error: A snapshot doesn't exist at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/snapshots/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts-snapshots/has-primary-styling-on-active-entry-1-chromium-linux.png, writing actual. 59 | 60 | await expect(navigation).toBeVisible() > 61 | await expect(item).toHaveScreenshot() | ^ 62 | }) 63 | 64 | test('has primary button styling on active entry with editing=true', { tag: '@visual' }, async ({ page }) => { at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:61:2
})

test('has primary button styling on active entry with editing=true', { tag: '@visual' }, async ({ page }) => {
Expand All @@ -71,7 +71,7 @@
await item.getByRole('button', { name: 'Edit item' }).click()
await expect(item.getByRole('textbox')).toBeVisible()

await expect(item).toHaveScreenshot()

Check failure on line 74 in tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:64:1 › has primary button styling on active entry with editing=true @visual

6) [chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:64:1 › has primary button styling on active entry with editing=true @visual Error: A snapshot doesn't exist at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/snapshots/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts-snapshots/has-primary-button-styling-on-active-entry-with-editing-true-1-chromium-linux.png, writing actual. 72 | await expect(item.getByRole('textbox')).toBeVisible() 73 | > 74 | await expect(item).toHaveScreenshot() | ^ 75 | }) 76 | 77 | test('has tertiary styling on non active entry', { tag: '@visual' }, async ({ page }) => { at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:74:2
})

test('has tertiary styling on non active entry', { tag: '@visual' }, async ({ page }) => {
Expand All @@ -79,5 +79,5 @@
const item = navigation.getByRole('listitem').filter({ hasText: 'Bar' })

await expect(navigation).toBeVisible()
await expect(item).toHaveScreenshot()

Check failure on line 82 in tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts

View workflow job for this annotation

GitHub Actions / merge-reports

[chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:77:1 › has tertiary styling on non active entry @visual

7) [chromium] › components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:77:1 › has tertiary styling on non active entry @visual Error: A snapshot doesn't exist at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/snapshots/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts-snapshots/has-tertiary-styling-on-non-active-entry-1-chromium-linux.png, writing actual. 80 | 81 | await expect(navigation).toBeVisible() > 82 | await expect(item).toHaveScreenshot() | ^ 83 | }) 84 | at /home/runner/work/nextcloud-vue/nextcloud-vue/tests/component/components/NcAppNavigationItem/NcAppNavigationItem.spec.ts:82:2
})
15 changes: 14 additions & 1 deletion tests/unit/components/NcAppNavigation/NcAppNavigation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,20 @@ describe('NcAppNavigation.vue', () => {
expect(navigation.attributes('aria-hidden')).toBe('true')
expect(navigation.attributes('inert')).toBeTruthy()
expect(togglebutton.attributes('aria-expanded')).toBe('false')
expect(togglebutton.attributes('aria-label')).toBe('Open navigation')
expect(togglebutton.attributes('aria-label')).toBe('Open navigation [n]')
})

it('has correct aria attributes and inert on closed navigation with disabled shortcuts', async () => {
window.OCP = { Accessibility: { disableKeyboardShortcuts: () => true } }
const wrapper = mount(NcAppNavigation)
const togglebutton = findToggleButton(wrapper)

// Close navigation
await togglebutton.trigger('click')
expect(togglebutton.attributes('aria-label')).toBe('Open navigation [n]')

// Clean up
delete window.OCP
})

it('has aria-label from corresponding prop on navigation', () => {
Expand Down
Loading