feat(NcAppNavigation): add n hotkey to toggle navigation

[skjnldsv]

Subset of nextcloud/server#30484

[skjnldsv]

/backport to next

[ChristophWurst]

nice

[ShGKme]

Nice!

Could you also add aria-keyshortcuts to the toggle button for accessibility as well as something like [n] to the button's title (tooltip)? So users can find the shortcut.

Should we also close navigation by n?

[skjnldsv]

Could you also add aria-keyshortcuts to the toggle button for accessibility

Sure :)

Should we also close navigation by n?

We could do escape instead, assuming if we're in focus within the navigation 🤔

[skjnldsv]

Done @ShGKme
Also added cypress tests

[skjnldsv]

Done @ShGKme

[ShGKme]

To not change strings. It doesn't change the string meaning, no translation is required.

Suggested change

	: t('Open navigation {shortcut}', { shortcut: disableKeyboardShortcuts ? '' : '[n]' })
	: t('Open navigation') + (disableKeyboardShortcuts ? '' : ' [n]')

[skjnldsv]

but it's not compatible with rtl languages then :)

[ShGKme]

Hmm, yes, because n is inherently LTR and not neutral like [], - or numbers, it is kept on the right on RTL.

But I'd still not add a new translation string for something that is not a text but a technical concatenation.

What about adding a utility to @nextcloud/l10n to concatenate strings taking RTL into account? So we never need to ask many translators to translate such things.

[skjnldsv]

Done @ShGKme

• if closed, pressing n will open it
• if opened AND focus within, pressing n will close it
• if opened and focus NOT within, pressing n will do nothing

[ShGKme]

So there is no way to close navigation with a hotkey without putting the focus on the navigation first?

[skjnldsv]

So there is no way to close navigation with a hotkey without putting the focus on the navigation first?

I think that would lead to issues otherwise