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

fix(v2): fix some uncaught a11y issues in public form, login page #3875

Merged
merged 4 commits into from
May 19, 2022

Conversation

karrui
Copy link
Contributor

@karrui karrui commented May 17, 2022

Problem

Fixes some a11y issues raised in design review of #3861 (chromatic link)

Solution

Bug Fixes:

  • fix(LoginPage): remove extra htmlFor prop
  • fix(ColumnCell): use VisuallyHidden rather than display none
  • feat(AuthImageSvgr): update auth svg to a more neutral color

Before & After Screenshots

Public form and login page stories should have (less) a11y violations

karrui added 3 commits May 17, 2022 23:16
chakraUI already adds the correct props when nested in a FormControl component. The htmlFor prop is actually the wrong id.
so label can still be used for aria-labelledby
@karrui karrui requested a review from timotheeg May 17, 2022 15:26
@karrui karrui changed the title fix(v2): fix some uncaught a11y issues in public form fix(v2): fix some uncaught a11y issues in public form, login page May 17, 2022
@karrui karrui requested a review from tshuli May 17, 2022 15:27
Copy link
Contributor

@timotheeg timotheeg left a comment

Choose a reason for hiding this comment

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

Needs to resolve conflicts

# Conflicts:
#	frontend/src/features/login/components/LoginForm.tsx
@karrui karrui merged commit b946cce into form-v2/develop May 19, 2022
@karrui karrui deleted the form-v2/fix-public-form-a11y branch May 19, 2022 01:31
@justynoh justynoh mentioned this pull request Oct 5, 2022
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.

2 participants