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.

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

added: [M3-6989] - aria-describedby to TextField with helper text #11351

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Dec 2, 2024

Description 📝

Tiny PR to add better accessibility to the Textfield via its helperText prop.

On screen, it is trivial for a user to see a helper test associated with a field, but this is not necessarily true for someone using a screen reader, since the helper text is a plain paragraph floating under the input, and isn't associated with its relevant interactive element.

Changes 🔄

  • Describe the textfield by its helper text when disabled
  • Cleanup textfield input ID logic (see self review)

Preview 📷

Screenshot 2024-12-02 at 16 39 25

How to test 🧪

Prerequisites

Verification steps

  • Check markup to confirm Textfield & HelperText relationship
  • Optional: confirm behavior with VoiceOver util

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@abailly-akamai abailly-akamai self-assigned this Dec 2, 2024
@abailly-akamai abailly-akamai added the Accessibility Contains accessibility improvements or changes label Dec 2, 2024
@@ -113,7 +113,8 @@ interface InputToolTipProps {
tooltipWidth?: number;
}

interface TextFieldPropsOverrides extends StandardTextFieldProps {
interface TextFieldPropsOverrides
extends Omit<StandardTextFieldProps, 'label'> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This helps TS in general not having to compare overridden types. While in this case this is just a string and this likely has no impact, it's just good practice.

(label
? convertToKebabCase(label)
: // label could still be an empty string
fallbackId);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

always making sure we have an ID

@abailly-akamai abailly-akamai changed the title change: [M3-6989] Add aria-describedby to TextField with helper text added: [M3-6989] Add aria-describedby to TextField with helper text Dec 3, 2024
@abailly-akamai abailly-akamai changed the title added: [M3-6989] Add aria-describedby to TextField with helper text added: [M3-6989] - aria-describedby to TextField with helper text Dec 3, 2024
@abailly-akamai abailly-akamai marked this pull request as ready for review December 3, 2024 02:50
@abailly-akamai abailly-akamai requested a review from a team as a code owner December 3, 2024 02:50
@abailly-akamai abailly-akamai requested review from dwiley-akamai and hkhalil-akamai and removed request for a team December 3, 2024 02:50
Copy link

github-actions bot commented Dec 3, 2024

Coverage Report:
Base Coverage: 86.83%
Current Coverage: 86.83%

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 1 failing test on test run #6 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing461 Passing2 Skipped103m 14s

Details

Failing Tests
SpecTest
clone-linode.spec.tsclone linode » can clone a Linode from Linode details page

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/linodes/clone-linode.spec.ts"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Contains accessibility improvements or changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants