enh(NcContent): Add skip content buttons #4983

susnux · 2023-12-21T11:59:55Z

☑️ Resolves

Fix [BITV]: Skip links are not working anymore nextcloud/server#41696

This adds skip content buttons for the Vue app when mounting NcContent.
It also handles the app navigation if it is not available (meaning do not add that button if there is no app navigation).
And it opens the app navigation when skipping to it.

🏁 Checklist

⛑️ Tests are included or are not applicable
📘 Component documentation has been extended, updated or is not applicable

src/components/NcAppNavigation/NcAppNavigation.vue

src/components/NcContent/NcContent.vue

JuliaKirschenheuter

Thank you for taking over!

Unfortunately it is not working well (tested again after your second push). 2 scenarios:

Check "Skip to app navigation" one time -> navigate a bit -> check "Skip to app navigation" second time -> focus remains on a skip button and nothing happens.
Check "Skip to app navigation" -> go to the app navigation and try to navigate backwards (Shift + Tab) -> navigation will move to the header, not to the app navigation

JuliaKirschenheuter · 2023-12-21T15:35:34Z

But those scenarios #4983 (review) are working for "skip to main content" well

susnux · 2023-12-21T15:40:59Z

Check "Skip to app navigation" one time -> navigate a bit -> check "Skip to app navigation" second time -> focus remains on a skip button and nothing happens.

Ah I see when the location hash is already set to that ID then the browser will not update the focus. I fixed this now ✔️

susnux · 2023-12-21T15:43:13Z

Check "Skip to app navigation" -> go to the app navigation and try to navigate backwards (Shift + Tab) -> navigation will move to the header, not to the app navigation

What do you mean? If you check "skip to navigation" then the focus should be on the navigation. If you press shift-tab then the next element in reverse order is focused (probably the settings button in the app menu?).

ShGKme · 2023-12-21T15:55:09Z

2. Check "Skip to app navigation" -> go to the app navigation and try to navigate backwards (Shift + Tab) -> navigation will move to the header, not to the app navigation

"Skip buttons" supposed to be one way — to skip the header or navigation. But user doesn't need to return to them on backward navigation.

In screen readers a user can return to the beginning with a special shortcut (CTRL+Home usually)

ShGKme · 2023-12-21T15:57:12Z

src/components/NcAppNavigation/NcAppNavigation.vue

+	inject: {
+		setHasAppNavigation: {
+			default: () => () => Vue.util.warn('NcAppNavigation is not mounted inside NcContent, this is probably an error.'),


If we show a warning here, we should add something to the Files app to not show the warning

I would say it depends. Is there any good reason to not use NcContent?
If yes: Then we can remove the warning and adjust the docs.
If not: We should adjust files to use it.

There isn't, this is the only way we ask Devs to build Vue apps

susnux · 2023-12-21T21:07:53Z

@nextcloud-libraries/designers this is unrelated to this PR (also on current master) but maybe we can take the opportunity and improve it.
When having the app navigation on an app the skip content buttons are hard to see because they are colored primary like the active navigation element element. Maybe we should add some background to the buttons to make them more visual distinguishable?

With background	current

marcoambrosini · 2023-12-21T23:52:56Z

Good idea @susnux! Then I would suggest making it even bigger and more informative. I sketched out a suggestion.
I can provide SVGs for the images of the focused parts. What do you think @nextcloud-libraries/designers ?

Todo:

Invert order so that UI section and buttons are vertically aligned
Possibly gray out UI and use color in the highlighted part. The highlighted section should stand out more

susnux · 2023-12-22T10:11:20Z

@marcoambrosini that looks really amazing! Should we directly implement this or first fix this issue here and the improve when the SVGs are ready?

JuliaKirschenheuter · 2023-12-22T10:22:10Z

@marcoambrosini would it be possible to backport so large improvement to stable28?

ShGKme · 2023-12-22T10:50:22Z

@marcoambrosini would it be possible to backport so large improvement to stable28?

I think this is not large as this part is visible only on initial keyboard navigation.

JuliaKirschenheuter

skip links are working fine. Can't see SVG's

marcoambrosini · 2024-01-18T13:41:45Z

It looks like the svg is getting resized?

susnux · 2024-01-18T16:58:06Z

It looks like the svg is getting resized?

Forgot to set the size correctly, done now ✔️

susnux · 2024-01-18T16:58:10Z

marcoambrosini

Nice, just a couple of comments about the buttons:

I would invert the order so that first comes navigation and then content
I would make both of them tertiary

ShGKme · 2024-01-19T11:49:02Z

I would make both of them tertiary

Maybe secondary is better here? To be more visible

ShGKme · 2024-01-19T11:52:33Z

Primary	Secondary	Tertiary

marcoambrosini · 2024-01-19T12:23:59Z

@ShGKme since one f the two will always have a border, I don't think it's needed. But I'm fine with either.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Images created by Marco Ambrosini Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux · 2024-01-19T16:02:22Z

Nice, just a couple of comments about the buttons

I pushed a commit to resolve both points.

vokoscreenNG-2024-01-19_17-01-45.mp4

susnux added enhancement New feature or request 3. to review Waiting for reviews feature: app-content Related to the app-content component labels Dec 21, 2023

susnux requested review from raimund-schluessler, JuliaKirschenheuter, skjnldsv and ShGKme December 21, 2023 11:59

susnux marked this pull request as ready for review December 21, 2023 12:00

susnux force-pushed the enh/skip-buttons branch from b0374a9 to 9c922a6 Compare December 21, 2023 12:03

susnux mentioned this pull request Dec 21, 2023

[BITV]: Skip links are not working anymore nextcloud/server#41696

Closed

JuliaKirschenheuter assigned susnux Dec 21, 2023

ShGKme reviewed Dec 21, 2023

View reviewed changes

susnux force-pushed the enh/skip-buttons branch 3 times, most recently from cb74159 to cdc6a7f Compare December 21, 2023 15:29

JuliaKirschenheuter requested changes Dec 21, 2023

View reviewed changes

susnux force-pushed the enh/skip-buttons branch from cdc6a7f to 3025247 Compare December 21, 2023 15:40

ShGKme reviewed Dec 21, 2023

View reviewed changes

susnux force-pushed the enh/skip-buttons branch from 3025247 to dfe135b Compare December 21, 2023 18:18

susnux requested review from ShGKme and JuliaKirschenheuter December 21, 2023 18:18

susnux force-pushed the enh/skip-buttons branch from 8ae45c6 to 3405382 Compare January 15, 2024 12:38

This comment was marked as resolved.

Sign in to view

susnux force-pushed the enh/skip-buttons branch from 3405382 to a4ac5b1 Compare January 15, 2024 12:43

JuliaKirschenheuter approved these changes Jan 15, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

susnux force-pushed the enh/skip-buttons branch from a4ac5b1 to d081f21 Compare January 18, 2024 16:59

This comment was marked as resolved.

Sign in to view

ShGKme approved these changes Jan 18, 2024

View reviewed changes

marcoambrosini reviewed Jan 19, 2024

View reviewed changes

susnux added 5 commits January 19, 2024 16:39

enh(NcContent): Add skip content buttons

f482cf2

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

enh(NcIconSvgWrapper): Allow to set size using a size prop

c493259

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

enh(NcContent): Improve skip-buttons design by adding helper images

8dc135f

Images created by Marco Ambrosini Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(NcContent): Scope styles to vue apps

cf9ddb5

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(NcContent): Adjust skip-content-buttons as suggested by designers

dc5ef51

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux force-pushed the enh/skip-buttons branch from de5504d to dc5ef51 Compare January 19, 2024 15:39

susnux requested a review from marcoambrosini January 19, 2024 16:02

ShGKme approved these changes Jan 19, 2024

View reviewed changes

susnux merged commit 808bbb2 into master Jan 20, 2024
16 checks passed

susnux deleted the enh/skip-buttons branch January 20, 2024 03:06

Pytal mentioned this pull request Jan 23, 2024

chore(release): v8.5.0 #5113

Merged

ShGKme mentioned this pull request Jan 24, 2024

fix(files): combine navigation and files list into single Vue app nextcloud/server#43090

Merged

7 tasks

ShGKme mentioned this pull request Feb 1, 2024

chore(deps): Bump @nextcloud/vue from 8.4.0 to 8.6.0 nextcloud/talk-desktop#505

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enh(NcContent): Add skip content buttons #4983

enh(NcContent): Add skip content buttons #4983

susnux commented Dec 21, 2023 •

edited

JuliaKirschenheuter left a comment

JuliaKirschenheuter commented Dec 21, 2023

susnux commented Dec 21, 2023

susnux commented Dec 21, 2023

ShGKme commented Dec 21, 2023

ShGKme Dec 21, 2023

susnux Dec 21, 2023

skjnldsv Dec 22, 2023

susnux commented Dec 21, 2023

marcoambrosini commented Dec 21, 2023 •

edited

susnux commented Dec 22, 2023

JuliaKirschenheuter commented Dec 22, 2023

ShGKme commented Dec 22, 2023

This comment was marked as resolved.

This comment was marked as resolved.

JuliaKirschenheuter left a comment

This comment was marked as resolved.

marcoambrosini commented Jan 18, 2024

susnux commented Jan 18, 2024

susnux commented Jan 18, 2024

This comment was marked as resolved.

marcoambrosini left a comment

ShGKme commented Jan 19, 2024 •

edited

ShGKme commented Jan 19, 2024 •

edited

marcoambrosini commented Jan 19, 2024

susnux commented Jan 19, 2024

enh(NcContent): Add skip content buttons #4983

enh(NcContent): Add skip content buttons #4983

Conversation

susnux commented Dec 21, 2023 • edited

☑️ Resolves

🏁 Checklist

JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

JuliaKirschenheuter commented Dec 21, 2023

susnux commented Dec 21, 2023

susnux commented Dec 21, 2023

ShGKme commented Dec 21, 2023

ShGKme Dec 21, 2023

Choose a reason for hiding this comment

susnux Dec 21, 2023

Choose a reason for hiding this comment

skjnldsv Dec 22, 2023

Choose a reason for hiding this comment

susnux commented Dec 21, 2023

marcoambrosini commented Dec 21, 2023 • edited

susnux commented Dec 22, 2023

JuliaKirschenheuter commented Dec 22, 2023

ShGKme commented Dec 22, 2023

This comment was marked as resolved.

This comment was marked as resolved.

JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

marcoambrosini commented Jan 18, 2024

susnux commented Jan 18, 2024

susnux commented Jan 18, 2024

This comment was marked as resolved.

marcoambrosini left a comment

Choose a reason for hiding this comment

ShGKme commented Jan 19, 2024 • edited

ShGKme commented Jan 19, 2024 • edited

marcoambrosini commented Jan 19, 2024

susnux commented Jan 19, 2024

susnux commented Dec 21, 2023 •

edited

marcoambrosini commented Dec 21, 2023 •

edited

ShGKme commented Jan 19, 2024 •

edited

ShGKme commented Jan 19, 2024 •

edited