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

[BITV/A11Y] NcAppNavigationNewItem should be implemented as button not span. #4108

Closed
nfebe opened this issue May 13, 2023 · 0 comments · Fixed by #4151
Closed

[BITV/A11Y] NcAppNavigationNewItem should be implemented as button not span. #4108

nfebe opened this issue May 13, 2023 · 0 comments · Fixed by #4151
Assignees
@nfebe
Labels
accessibility Making sure we design for the widest range of people possible, including those who have disabilities design Design, UX, interface and interaction design discussion Need advices, opinions or ideas on this topic enhancement New feature or request

Comments

@nfebe
Copy link
Contributor

nfebe commented May 13, 2023

Following nextcloud/server #37135 it appears the right NcAppNavigationItem to use would be NcAppNavigationNewItem. That's better than using an anchor tag as it was with NcAppNavigationItem. However, NcAppNavigationNewItem by virtue of how it functions triggers and action without routing anywhere so it should be semantically implemented as a button.

Code : https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/NcAppNavigationNewItem/NcAppNavigationNewItem.vue

https://github.com/nextcloud/nextcloud-vue/blob/2d6245af80f9cfc5004d67489d00a4b22774e350/src/components/NcAppNavigationNewItem/NcAppNavigationNewItem.vue#L79-L81

Helpful Notes : https://karlgroves.com/links-are-not-buttons-neither-are-divs-and-spans/
@nfebe nfebe added enhancement New feature or request design Design, UX, interface and interaction design discussion Need advices, opinions or ideas on this topic accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels May 13, 2023
nfebe added a commit to nextcloud/server that referenced this issue May 13, 2023
@nfebe
Convert Add group link to button
140d0c8 
The "Add group" peforms an on-page action and does not route or link to
anywhere else hence, not semantically a link but a button.

This commit implements the ehancement described at the respository level.

A change would be required in [@nextcloud/nextcloud-vue](nextcloud-libraries/nextcloud-vue#4108)
since the `NcAppNavigationNewItem` is an out-of-repo dependency.

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit to nextcloud/server that referenced this issue May 13, 2023
@nfebe
Convert Add group link to button
473f54c 
The "Add group" peforms an on-page action and does not route or link to
anywhere else hence, not semantically a link but a button.

This commit implements the ehancement described at the respository level.

A change would be required in [@nextcloud/nextcloud-vue](nextcloud-libraries/nextcloud-vue#4108)
since the `NcAppNavigationNewItem` is an out-of-repo dependency.

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit to nextcloud/server that referenced this issue May 13, 2023
@nfebe
Convert Add group link to button
b862829 
The "Add group" peforms an on-page action and does not route or link to
anywhere else hence, not semantically a link but a button.

This commit implements the ehancement described at the respository level.

A change would be required in [@nextcloud/nextcloud-vue](nextcloud-libraries/nextcloud-vue#4108)
since the `NcAppNavigationNewItem` is an out-of-repo dependency.

Signed-off-by: fenn-cs <[email protected]>
@nfebe nfebe mentioned this issue May 13, 2023
Merged
1 task
nfebe added a commit to nextcloud/server that referenced this issue May 14, 2023
@nfebe
Convert Add group link to button
1ee8fba 
The "Add group" peforms an on-page action and does not route or link to
anywhere else hence, not semantically a link but a button.

This commit implements the ehancement described at the respository level.

A change would be required in [@nextcloud/nextcloud-vue](nextcloud-libraries/nextcloud-vue#4108)
since the `NcAppNavigationNewItem` is an out-of-repo dependency.

Signed-off-by: fenn-cs <[email protected]>
@nfebe nfebe self-assigned this May 24, 2023
nfebe added a commit that referenced this issue May 24, 2023
@nfebe
Implement app menu new-item action as button not link
71d9a56 
`NcAppNavigationNewItem` performs an on-page action and so the correct sematic
tag is button.

Resolves : #4108

Signed-off-by: fenn-cs <[email protected]>
@nfebe nfebe mentioned this issue May 24, 2023
Merged
2 tasks
nfebe added a commit that referenced this issue May 30, 2023
@nfebe
Implement app menu new-item action as button not div
4593d01 
`NcAppNavigationNewItem` performs an on-page action and so the correct sematic
tag is button.

Additionally, remove divs inside new button implementation.

Resolves : #4108

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue May 30, 2023
@nfebe
Implement app menu new-item action as button not div
4ae000c 
`NcAppNavigationNewItem` performs an on-page action and so the correct sematic
tag is button.

Additionally, remove divs inside new button implementation.

Resolves : #4108

Signed-off-by: fenn-cs <[email protected]>
@nfebe nfebe changed the title NcAppNavigationNewItem should be implemented as button not span. [BITV/A11Y] NcAppNavigationNewItem should be implemented as button not span. May 30, 2023
nfebe added a commit that referenced this issue May 31, 2023
@nfebe
Implement app menu new-item action as button not div
d21154e 
`NcAppNavigationNewItem` performs an on-page action and so the correct sematic
tag is button.

Additionally, remove divs inside new button implementation.

Resolves : #4108

Signed-off-by: fenn-cs <[email protected]>
nfebe added a commit that referenced this issue May 31, 2023
@nfebe
Implement app menu new-item action as button not div
de81c14 
`NcAppNavigationNewItem` performs an on-page action and so the correct sematic
tag is button.

Additionally, remove divs inside new button implementation.

Resolves : #4108

Signed-off-by: fenn-cs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nfebe nfebe

Labels
accessibility Making sure we design for the widest range of people possible, including those who have disabilities design Design, UX, interface and interaction design discussion Need advices, opinions or ideas on this topic enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement app menu new-item action as button not link nextcloud-libraries/nextcloud-vue
1 participant
@nfebe