Fix accessibility errors

[IshavSohal]

Related Item(s)

#362

Changes

• Fixed accessibly errors in
  • Main editor page
  • New product page
  • Edit existing product page
  • Choose language page
  • Create/Edit product option page

Notes

• There are still a few accessibility issues remaining, which I believe are all issues with the highcharts editor

Testing

Steps:

1. Open storylines to the 'choose language page'. Open the WAVE extension and observe that there are no (non-highcharts) errors
2. Click English to open the 'create/edit product option' page. Open the WAVE extension and observe that there are no (non-highcharts) errors
3. Click on the 'Create New Storylines Product' button to open the 'new product page'. Open the WAVE extension and observe that there are no (non-highcharts) errors
4. Go back to the 'create/edit product option page'
5. Click on the 'Edit Existing Storylines Product Button' to open the 'edit existing product' page. Enter an uuid of:

00000000-0000-0000-0000-000000000000

6. Open the WAVE extension and observe that there are no (non-highcharts) errors
7. Click the 'Load' button. Open the WAVE extension and observe that there are no (non-highcharts) errors
8. Click the 'Next' button to open the main editor page. Open the WAVE extension and observe that there are no (non-highcharts) errors

---

This change is 

[github-actions]

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/issue-362-fix

[yileifeng]

Few more errors generated using WAVE (many of these are the same error across different editor types):

Image editor

• low contrast on "browse" (in file upload area)
• empty button on delete icon
• missing form labels (alt tag input, image caption input, slideshow caption)

Dynamic editor

• empty table header for the add new panel row
• missing form label on the "Enter Panel ID" input

Advanced editor

• low contrast on switch editor mode button

RAMP editor (issue for RAMP config repo)

• missing form labels for all inputs
• many empty buttons for all information mini-icons

Chart editor

• low contrast on "Empty" text when there are no charts
• empty button on delete icon

Video editor

• low contrast on "browse" (same as image editor)
• missing form labels (video title, URL input)
• empty button on delete icon

Slideshow editor

• missing form label (slideshow caption)
• empty table header (on table header row)

Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions (waiting on @IshavSohal)

---

src/components/editor.vue line 22 at r1 (raw file):

                            transform="translate(-0.5)"
                        />
                        .

I think instead of doing this adding an aria-label attribute to the element would be a better fix

---

src/components/slide-toc.vue line 29 at r1 (raw file):

                        d="M5 22q-.825 0-1.413-.587Q3 20.825 3 20V6h2v14h11v2Zm4-4q-.825 0-1.412-.587Q7 16.825 7 16V4q0-.825.588-1.413Q8.175 2 9 2h9q.825 0 1.413.587Q20 3.175 20 4v12q0 .825-.587 1.413Q18.825 18 18 18Zm0-2h9V4H9v12Zm0 0V4v12Z"
                    />
                    -

same as earlier, try aria-label

[IshavSohal]

Thanks for the help. I was able to fix all of the errors you listed (apart from the RAMP config editor ones) except for the 'low contrast' error on the switch editor mode button within the advanced editor. I think it's because this button is a part of the json-editor component (within custom-editor.vue).

Reviewable status: 0 of 15 files reviewed, 2 unresolved discussions (waiting on @yileifeng)

---

src/components/editor.vue line 22 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

I think instead of doing this adding an aria-label attribute to the element would be a better fix

Good idea, donethanks

---

src/components/slide-toc.vue line 29 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

same as earlier, try aria-label

Donethanks

[yileifeng]

Most of the editor specific WAVE errors got cleared so nice work, will make a new issue for the RAMP config accessibility errors. Just one more error popping up on the video editor:

• missing form label on the paste URL input for video editor

Reviewable status: 0 of 15 files reviewed, 2 unresolved discussions

[spencerwahl]

All of the labels and such should be put in the lang csv so french can be added as well

Reviewable status: 0 of 15 files reviewed, 8 unresolved discussions (waiting on @IshavSohal and @yileifeng)

---

src/components/dynamic-editor.vue line 54 at r2 (raw file):

                    <th class="flex flex-col items-center" aria-label="Enter Panel ID">
                        <label for="panelId" aria-label="Enter Panel ID"></label>
                        <input

If the label isn't being made visible, there should just be an aria-label directly on the input element. I also don't believe the table header needs an aria-label as it is not focusable.

---

src/components/editor.vue line 22 at r1 (raw file):

Previously, IshavSohal (Ishav Sohal) wrote…

Good idea, donethanks

The aria-label should go on the focusable element here, which should be the router-link

---

src/components/image-editor.vue line 55 at r2 (raw file):

                        <label class="editor-label text-label" for="altTag">{{ $t('editor.image.altTag') }}:</label>
                        <input
                            id="altTag"

Because the alt tag and image caption inputs appear for each image, these ids get duplicated, which will cause issues. I would append the index or the id above or some otherwise differentiating string to the end of the id.

---

src/components/slide-toc.vue line 29 at r1 (raw file):

Previously, IshavSohal (Ishav Sohal) wrote…

Donethanks

same as above, aria-label should move to the button

---

src/components/slideshow-editor.vue line 55 at r2 (raw file):

            <thead>
                <tr class="table-header">
                    <th aria-label="empty header"></th>

The labels should be useful info, I would label this "slide number" or "index" or something similar

---

src/components/helpers/chart-preview.vue line 16 at r2 (raw file):

                    viewBox="0 0 352 512"
                    xmlns="http://www.w3.org/2000/svg"
                    :aria-label="$t('editor.chart.delete')"

same as above for both of these svg labels

---

src/components/helpers/image-preview.vue line 15 at r2 (raw file):

                    viewBox="0 0 352 512"
                    xmlns="http://www.w3.org/2000/svg"
                    :aria-label="$t('editor.image.delete')"

again

---

src/components/helpers/video-preview.vue line 10 at r2 (raw file):

                v-tippy="{ placement: 'top', hideOnClick: false, animateFill: true }"
            >
                <svg height="24px" width="24px" viewBox="0 0 352 512" xmlns="http://www.w3.org/2000/svg" :aria-label="$t('editor.video.delete')">

again

[IshavSohal]

Donethanks. I think I got all of them, but let me know if I've missed any

Reviewable status: 0 of 16 files reviewed, 8 unresolved discussions (waiting on @spencerwahl and @yileifeng)

---

src/components/dynamic-editor.vue line 54 at r2 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

If the label isn't being made visible, there should just be an aria-label directly on the input element. I also don't believe the table header needs an aria-label as it is not focusable.

Donethanks

---

src/components/editor.vue line 22 at r1 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

The aria-label should go on the focusable element here, which should be the router-link

Donethanks

---

src/components/image-editor.vue line 55 at r2 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

Because the alt tag and image caption inputs appear for each image, these ids get duplicated, which will cause issues. I would append the index or the id above or some otherwise differentiating string to the end of the id.

Donethanks

---

src/components/slide-toc.vue line 29 at r1 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

same as above, aria-label should move to the button

Donethanks

---

src/components/slideshow-editor.vue line 55 at r2 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

The labels should be useful info, I would label this "slide number" or "index" or something similar

Donethanks

---

src/components/helpers/chart-preview.vue line 16 at r2 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

same as above for both of these svg labels

Donethanks

---

src/components/helpers/image-preview.vue line 15 at r2 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

again

Donethanks

---

src/components/helpers/video-preview.vue line 10 at r2 (raw file):

Previously, spencerwahl (Spencer Wahl) wrote…

again

Donethanks

[IshavSohal]

Most of the editor specific WAVE errors got cleared so nice work, will make a new issue for the RAMP config accessibility errors. Just one more error popping up on the video editor:

• missing form label on the paste URL input for video editor

Reviewable status: 0 of 15 files reviewed, 2 unresolved discussions

Sorry, forgot to respond to this earlier. I'm not getting this error on my end. Is it still occurring for you?

[yileifeng]

Just re-tested and seems like it:

Reviewable status: 0 of 16 files reviewed, 8 unresolved discussions (waiting on @spencerwahl)

[IshavSohal]

Should be fixed now.

[yileifeng]

Reviewed 4 of 7 files at r1, 1 of 10 files at r2, 8 of 11 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @spencerwahl)

[yileifeng]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @spencerwahl)

[RyanCoulsonCA]

Looks good! I've just caught a few more small things.

Reviewed 4 of 7 files at r1, 1 of 10 files at r2, 7 of 11 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @IshavSohal, @spencerwahl, and @yileifeng)

---

src/components/dynamic-editor.vue line 53 at r5 (raw file):

                <tr class="table-add-row">
                    <th class="flex flex-col items-center">
                        <label for="panelId"></label>

This is empty so we don't actually need it at all. The aria-label on the input below is good enough!

---

src/components/slide-editor.vue line 244 at r5 (raw file):

                    <span class="ml-auto flex-grow"></span>
                    <div v-if="!advancedEditorView || rightOnly" class="flex flex-col mr-8">
                        <label class="editor-label text-left text-lg">{{ $t('editor.slides.contentType') }}:</label>

This guy needs a for attribute as well.

---

src/components/text-editor.vue line 5 at r5 (raw file):

        <label class="editor-label text-left" for="panelTitle">{{ $t('editor.slides.panel.title') }}:</label>
        <input class="editor-input" type="text" id="panelTitle" v-model="panel.title" />
        <label class="editor-label text-left mt-2">{{ $t('editor.slides.panel.body') }}:</label>

Needs a `for attribute as well.

---

src/components/helpers/metadata-content.vue line 34 at r5 (raw file):

        <!-- only display an image preview if one is provided.-->
        <div v-if="!!metadata.logoPreview">
            <label class="editor-label">{{ $t('editor.logoPreview') }}:</label>

This guy needs a for attribute like the others. It currently gives an orphaned form label warning in WAVE.

---

src/components/helpers/metadata-content.vue line 39 at r5 (raw file):

                v-if="!!metadata.logoPreview && metadata.logoPreview != 'error'"
                class="image-preview"
                alt="preview of product logo"

This should have a translation

[IshavSohal]

Reviewable status: 11 of 16 files reviewed, 13 unresolved discussions (waiting on @RyanCoulsonCA, @spencerwahl, and @yileifeng)

---

src/components/dynamic-editor.vue line 53 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This is empty so we don't actually need it at all. The aria-label on the input below is good enough!

Donethanks. I also ended up adding the aria label back to the table header because I was getting an 'empty table header' error.

---

src/components/slide-editor.vue line 244 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This guy needs a for attribute as well.

Donethanks

---

src/components/text-editor.vue line 5 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

Needs a `for attribute as well.

Since this label didn't have a corresponding form control, I just made it a div instead. Is this fine?

---

src/components/helpers/metadata-content.vue line 34 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This guy needs a for attribute like the others. It currently gives an orphaned form label warning in WAVE.

Also ended up making this a div.

---

src/components/helpers/metadata-content.vue line 39 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This should have a translation

Donethanks

[RyanCoulsonCA]

Reviewed 1 of 10 files at r7, all commit messages.
Reviewable status: 5 of 16 files reviewed, 10 unresolved discussions (waiting on @IshavSohal, @spencerwahl, and @yileifeng)

---

src/components/dynamic-editor.vue line 53 at r5 (raw file):

Previously, IshavSohal (Ishav Sohal) wrote…

Donethanks. I also ended up adding the aria label back to the table header because I was getting an 'empty table header' error.

Whoops this is actually a mistake on my part when I implemented this feature. These cells should be td, not th. The only place we need th is in the first row. Just swap these with td (and change the .table-add-row th CSS selector at the bottom of the file to .table-add-row td), remove that aria-label and you shouldn't get an error anymore :)

---

src/components/text-editor.vue line 5 at r5 (raw file):

Previously, IshavSohal (Ishav Sohal) wrote…

Since this label didn't have a corresponding form control, I just made it a div instead. Is this fine?

Yup, should be ok!

---

src/components/helpers/metadata-content.vue line 134 at r7 (raw file):

            <option value="horizontal">{{ $t('editor.tocOrientation.horizontal') }}</option>
        </select>
        <label class="editor-label ml-25">{{ $t('editor.returnTop') }}</label>

This one isn't your fault either, but this PR just got merged in and we let another accessibility error slip through 🤦. Should just need a for like the ToC orientation checkbox.

[IshavSohal]

Reviewable status: 5 of 16 files reviewed, 10 unresolved discussions (waiting on @RyanCoulsonCA, @spencerwahl, and @yileifeng)

---

src/components/dynamic-editor.vue line 53 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

Whoops this is actually a mistake on my part when I implemented this feature. These cells should be td, not th. The only place we need th is in the first row. Just swap these with td (and change the .table-add-row th CSS selector at the bottom of the file to .table-add-row td), remove that aria-label and you shouldn't get an error anymore :)

All good, donethanks

---

src/components/helpers/metadata-content.vue line 134 at r7 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This one isn't your fault either, but this PR just got merged in and we let another accessibility error slip through 🤦. Should just need a for like the ToC orientation checkbox.

Donethanks!

[RyanCoulsonCA]

Reviewed 1 of 5 files at r6, 8 of 10 files at r7, 2 of 2 files at r8, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @spencerwahl and @yileifeng)

[RyanCoulsonCA]

Reviewed all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @spencerwahl and @yileifeng)

[yileifeng]

Reviewed 1 of 5 files at r6, 9 of 10 files at r7, 2 of 2 files at r8, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @spencerwahl)

[spencerwahl]

Reviewed 3 of 11 files at r3, 1 of 1 files at r5, 1 of 5 files at r6, 9 of 10 files at r7, 2 of 2 files at r8, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @RyanCoulsonCA and @yileifeng)

[yileifeng]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA)

[RyanCoulsonCA]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)