Skip to content

Conversation

@gosiexon-zen
Copy link
Contributor

@gosiexon-zen gosiexon-zen commented Nov 4, 2025

Description

The WYSIWYG editor (CKEditor) is likely overriding the label association with its own aria-label="Editor editing area...", which violates WCAG 1.3.1. I tried using aria-labelledby but it was also overridden by Garden's default label ID. So I decided to use aria-label for textarea. I needed to update help-center-wysiwyg AriaAttributesPlugin so it copies ARIA attributes from the original textarea element. Now when you focus on the editor, it will maintain "Description editing area" instead of being overridden with "Editor editing area: main. Press ⌥0 for help."

Screenshots

When not focused:
Screenshot 2025-11-04 at 16 22 46

When focused:
Screenshot 2025-11-04 at 16 22 59

Screen.Recording.2025-11-04.at.16.19.22.mov

[Jira issue] https://zendesk.atlassian.net/browse/GA11YFIX-454

Checklist

  • 📗 all commit messages follow the conventional commits standard
  • ⬅️ changes are compatible with RTL direction
  • ♿ Changes to the UI are tested for accessibility and compliant with WCAG 2.1.
  • 📝 changes are tested in Chrome, Firefox, Safari and Edge
  • 📱 changes are responsive and tested in mobile
  • 👍 PR is approved by @zendesk/vikings

@gosiexon-zen gosiexon-zen requested review from a team as code owners November 4, 2025 16:07
@gosiexon-zen gosiexon-zen force-pushed the mbien/request-form-a11y-issue branch from 15986c7 to 000c0c4 Compare November 5, 2025 09:13
@gosiexon-zen gosiexon-zen force-pushed the mbien/request-form-a11y-issue branch 2 times, most recently from 4275f24 to a0e78b4 Compare November 5, 2025 13:30
@jgorfine-zendesk jgorfine-zendesk requested a review from a team November 5, 2025 16:10
Copy link
Contributor

@jgorfine-zendesk jgorfine-zendesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @gosiexon-zen, thanks for doing this work!

I'm requesting changes because I'm noticing that, as is, we're dropping the instructional text that lets screen reader users know about the keyboard shortcuts menu: Press ⌥0 for help. Since we have no other text on screen that gives these same instructions — or even lets someone know about the shortcuts dialog, in the first place — it's important that we keep this text in.

For context, this is what appears when someone presses ⌥0:

Image

Comment on lines +295 to +299
- translation:
key: "new-request-form.description.aria_label"
title: "[A11Y] Aria label value for the description field"
value: "Description editing area"
screenshot: "https://drive.google.com/file/d/1wDm8jmFjlUTkJlPq_OfHeJRCTanf5BOi/view?usp=drive_link"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we could also translate the instructions for viewing CKEditor's "Accessibility help" dialog. Though, I think we can do better than just Press ⌥0 for help, which says nothing about what kind of help we're offering. Why don't we say, instead, Press ⌥0 for keyboard shortcuts dialog?

I'm wondering if we'd also need to do some localization. Doing some quick Googling, it sounds like the Option key is similar to the AltGr key on some European keyboards. Does pressing AltGr + 0 achieve the same result as pressing Option + 0? If so, we might want to consider having the description text say that. 🤔 Let's check in with @olivia-tsao & co., on that.

Suggested change
- translation:
key: "new-request-form.description.aria_label"
title: "[A11Y] Aria label value for the description field"
value: "Description editing area"
screenshot: "https://drive.google.com/file/d/1wDm8jmFjlUTkJlPq_OfHeJRCTanf5BOi/view?usp=drive_link"
- translation:
key: "new-request-form.description.aria_label"
title: "[A11Y] Aria label value for the description field"
value: "Description editing area"
screenshot: "https://drive.google.com/file/d/1wDm8jmFjlUTkJlPq_OfHeJRCTanf5BOi/view?usp=drive_link"
- translation:
key: "new-request-form.description.aria_describedby"
title: "[A11Y] Aria-describedby value for the description field."
value: "Press ⌥0 for keyboard shortcuts dialog"
screenshot: "https://drive.google.com/file/d/..."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, it would be better if we could avoid overriding the default messages from CKEditor, and just add the "Description" label to what is already there.

I agree we could have a better message than "Press ⌥0 for help", on the other hand:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we don't want any override we don't have to do anything in Copenhagen Theme. Wysiwyg will now read label we already have here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gosiexon-zen Agree that shortcuts can be different for some langs depending on the keys used/ available in the language. In such cases, extracting the keys and asking translators to change according to the language can be helpful.

Comment on lines 60 to 92
<StyledField>
<Label>
{label}
{required && <Span aria-hidden="true">*</Span>}
</Label>
{description && (
<Hint dangerouslySetInnerHTML={{ __html: description }} />
)}
<Textarea
ref={ref}
name={name}
defaultValue={value as string}
validation={error ? "error" : undefined}
required={required}
onChange={(e) => onChange(e.target.value)}
rows={6}
isResizable
aria-label={ariaLabel}
/>
{error && <StyledMessage validation="error">{error}</StyledMessage>}
</StyledField>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make sure screen reader users are still able to access the Press ⌥0 for keyboard shortcuts dialog instruction. (See my other comment about improving the wording a little.) We can do this in 2 steps:

  1. Render the instructions inside of a visually-hidden element, which would be provided by the Garden Span component: <Span hidden>{text}</Span>
  2. Create an association between the <Textarea> and the <Span> via the aria-describedby attribute.

Here's how this might look, in the code:

import { useId } from '@zendeskgarden/container-utilities';
import { Span } from '@zendeskgarden/react-typography';

const descriptionId = useId();

<StyledField>
  ...
  <Textarea
    ...
    aria-label={ariaLabel}
    aria-describedby={descriptionId}
  />
  {error && <StyledMessage validation="error">{error}</StyledMessage>}
  {ariaDescribedby && <Span id={descriptionId} hidden>{ariaDescribedby}</Span>}
</StyledField>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that change the screen reader will read something like this, so probably we don't want this:
Screenshot 2025-11-06 at 12 21 47

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @gosiexon-zen — I see what you mean: our aria-describedby value is overriding the the hint text ID.

But! This doesn't have to be a problem. The aria-describedby attribute accepts multiple IDs as a value. So, we could do something like this:

<Textarea aria-describedby={`${hintId} ${instructionsId}`}>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants