Add accessible NcSelect and NcSelectTags components #3435

Pytal · 2022-11-05T00:03:22Z

Related changes

Fix styles Disable style override on vue-select elements nextcloud/server#35165
Fix eslint errors Bump @nextcloud/eslint-config from 8.1.2 to 8.1.3 #3474

https://github.com/sagalbot/vue-select follows many accessibility best practices as described in https://vue-select.org/guide/accessibility.html but will require upstream pull requests to further enhance accessibility

src/components/NcListItemIcon/NcListItemIcon.vue

src/components/NcMultiselect/NcEllipsisedOption.vue

src/components/NcMultiselectTags/NcMultiselectTags.vue

src/components/NcTimezonePicker/NcTimezonePicker.vue

src/components/NcListItemIcon/NcListItemIcon.vue

Pytal · 2022-11-19T00:40:43Z

New NcSelect and NcSelectTags components ready for code review with interactive examples at https://deploy-preview-3435--nextcloud-vue-components.netlify.app/#/Components/NcSelect

src/components/NcListItemIcon/NcIcon.vue

src/components/NcSelect/NcSelect.vue

src/components/NcSelectTags/api.js

Signed-off-by: Christopher Ng <chrng8@gmail.com>

- Import from '@mdi/svg/svg/icon.svg?raw' Signed-off-by: Christopher Ng <chrng8@gmail.com>

Signed-off-by: Christopher Ng <chrng8@gmail.com>

- Deprecate NcMultiselect component - Extract shared NcEllipsisedOption component Signed-off-by: Christopher Ng <chrng8@gmail.com>

- Deprecate NcMultiselectTags component Signed-off-by: Christopher Ng <chrng8@gmail.com>

Signed-off-by: Christopher Ng <chrng8@gmail.com>

artonge · 2022-11-23T09:21:14Z

src/components/NcListItemIcon/NcIconSvgWrapper.vue

+import logger from '../../utils/logger.js'
+
+export default {
+	name: 'NcIconSvgWrapper',


I think is makes sense to expose this component publicly. I had to use @skjnldsv/sanitize-svg, and I've seen a few other places where it was used too.
An opinion @skjnldsv ? with your expertise on your package and on icons :D

Any thoughts @skjnldsv?

Yep, you can have <script> inside an svg. So We need to sanitise them :)

👍 already sanitized @skjnldsv, in https://github.com/nextcloud/nextcloud-vue/blob/enh/a11y-multiselect/src/components/NcListItemIcon/NcIconSvgWrapper.vue#L68

@artonge was wondering whether or not we should expose this component publicly?

artonge

Good for me.

The comment about having NcIconSvgWrapper public can be ignored for merging, as we can always do it later :)

Pytal · 2022-11-23T17:06:40Z

Test component as sharing input in server

skjnldsv · 2022-11-25T06:49:17Z

src/components/NcListItemIcon/NcIconSvgWrapper.vue

+		renderHtmlString() {
+			if (!this.cleanSvg) {
+				return
+			}
+
+			const parser = new DOMParser()
+			const parsedDocument = parser.parseFromString(this.cleanSvg, 'image/svg+xml')
+
+			const errorNode = parsedDocument.querySelector('parsererror')
+			if (errorNode) {
+				logger.error(t('Error parsing svg'), errorNode)
+			}
+			const element = parsedDocument.documentElement
+			element.classList.add('icon-vue__svg')
+
+			if (this.title) {
+				const titleElement = document.createElement('title')
+				titleElement.textContent = this.title
+				if (element.firstElementChild) {
+					element.firstElementChild.prepend(titleElement)
+				}
+			}
+
+			this.htmlString = element.outerHTML
+		},


Isn't it going too far?
We can just target the svg tag within via .icon-vue svg

The primary purpose of this snippet is to add the accessible title element similar to https://github.com/robcresswell/vue-material-design-icons/blob/5.1.2/build.ts#L26 and thought that we might as well add the class as good practice due to the increased specificity of classes over tags

Not sure about performances though.
I think doing it from update() or mounted() and just appending the title or updating if already exists would be more efficient :)

But that can be a followup!

skjnldsv

see comments

Pytal added the 2. developing Work in progress label Nov 5, 2022

Pytal self-assigned this Nov 5, 2022

Pytal force-pushed the enh/a11y-multiselect branch from 092ae5e to 83ee0ff Compare November 8, 2022 02:52

Pytal mentioned this pull request Nov 8, 2022

[BITV/A11Y] Multiselect accessibility #2504

Closed

7 tasks

Pytal force-pushed the enh/a11y-multiselect branch 8 times, most recently from 5911015 to ce923be Compare November 16, 2022 02:40

Pytal changed the title ~~[WIP] Add accessible multiselect~~ Add accessible multiselect Nov 16, 2022

Pytal marked this pull request as ready for review November 16, 2022 02:52

Pytal added this to the 8.0.0 milestone Nov 16, 2022

Pytal requested review from skjnldsv, artonge and CarlSchwan November 16, 2022 02:53

This comment was marked as outdated.

Sign in to view

Pytal requested review from JuliaKirschenheuter and juliusknorr November 16, 2022 03:32

artonge reviewed Nov 16, 2022

View reviewed changes

skjnldsv requested changes Nov 17, 2022

View reviewed changes

src/components/NcListItemIcon/NcListItemIcon.vue Outdated Show resolved Hide resolved

skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 17, 2022

Pytal added the 3. to review Waiting for reviews label Nov 19, 2022

artonge requested changes Nov 21, 2022

View reviewed changes

Pytal added 8 commits November 22, 2022 16:29

Add vue-select

41b9bc6

Signed-off-by: Christopher Ng <chrng8@gmail.com>

Add @mdi/svg

efaebcd

- Import from '@mdi/svg/svg/icon.svg?raw' Signed-off-by: Christopher Ng <chrng8@gmail.com>

[TEMP] Disable style override on vue-select elements

ee4d240

Signed-off-by: Christopher Ng <chrng8@gmail.com>

Add icon svg and title support to NcListItemIcon

5189eb5

Signed-off-by: Christopher Ng <chrng8@gmail.com>

Add NcSelect component

ed6f3ed

- Deprecate NcMultiselect component - Extract shared NcEllipsisedOption component Signed-off-by: Christopher Ng <chrng8@gmail.com>

Add NcSelectTags component

84e7175

- Deprecate NcMultiselectTags component Signed-off-by: Christopher Ng <chrng8@gmail.com>

Add NcSelect section to style guide

24092ef

Signed-off-by: Christopher Ng <chrng8@gmail.com>

Fix unknown Actions and ActionButton component references

b700ce6

Signed-off-by: Christopher Ng <chrng8@gmail.com>

Pytal force-pushed the enh/a11y-multiselect branch from 3826acb to b700ce6 Compare November 23, 2022 00:29

Pytal modified the milestones: 8.0.0, 7.2.0 Nov 23, 2022

Pytal requested review from artonge and skjnldsv November 23, 2022 00:50

artonge reviewed Nov 23, 2022

View reviewed changes

artonge approved these changes Nov 23, 2022

View reviewed changes

skjnldsv reviewed Nov 25, 2022

View reviewed changes

skjnldsv requested changes Nov 25, 2022

View reviewed changes

Pytal requested a review from skjnldsv November 26, 2022 01:03

skjnldsv approved these changes Nov 29, 2022

View reviewed changes

skjnldsv merged commit 7e1904d into master Nov 29, 2022

skjnldsv deleted the enh/a11y-multiselect branch November 29, 2022 07:22

skjnldsv modified the milestones: 7.2.0, 7.1.0 Nov 29, 2022

Pytal mentioned this pull request Nov 30, 2022

[BITV] Share tab a11y issues nextcloud/server#33740

Closed

10 tasks

raimund-schluessler mentioned this pull request Dec 5, 2022

NcActions broken in docs #3542

Closed

Pytal mentioned this pull request Dec 9, 2022

[Tech Debt] Global interactive element styles override scoped Vue styles nextcloud/server#35690

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add accessible NcSelect and NcSelectTags components #3435

Add accessible NcSelect and NcSelectTags components #3435

Pytal commented Nov 5, 2022 •

edited

Loading

This comment was marked as outdated.

Pytal commented Nov 19, 2022

artonge Nov 23, 2022

Pytal Nov 24, 2022

skjnldsv Nov 25, 2022

Pytal Nov 26, 2022

artonge left a comment •

edited

Loading

Pytal commented Nov 23, 2022 •

edited

Loading

skjnldsv Nov 25, 2022

Pytal Nov 26, 2022 •

edited

Loading

skjnldsv Nov 29, 2022

skjnldsv left a comment

Add accessible NcSelect and NcSelectTags components #3435

Add accessible NcSelect and NcSelectTags components #3435

Conversation

Pytal commented Nov 5, 2022 • edited Loading

This comment was marked as outdated.

Pytal commented Nov 19, 2022

artonge Nov 23, 2022

Choose a reason for hiding this comment

Pytal Nov 24, 2022

Choose a reason for hiding this comment

skjnldsv Nov 25, 2022

Choose a reason for hiding this comment

Pytal Nov 26, 2022

Choose a reason for hiding this comment

artonge left a comment • edited Loading

Choose a reason for hiding this comment

Pytal commented Nov 23, 2022 • edited Loading

skjnldsv Nov 25, 2022

Choose a reason for hiding this comment

Pytal Nov 26, 2022 • edited Loading

Choose a reason for hiding this comment

skjnldsv Nov 29, 2022

Choose a reason for hiding this comment

skjnldsv left a comment

Choose a reason for hiding this comment

Pytal commented Nov 5, 2022 •

edited

Loading

artonge left a comment •

edited

Loading

Pytal commented Nov 23, 2022 •

edited

Loading

Pytal Nov 26, 2022 •

edited

Loading