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

[Chore] Accessibility fixes #111

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

Tanya-Amber-L
Copy link
Contributor

@Tanya-Amber-L Tanya-Amber-L commented Jan 29, 2025

Context

Update formango version
Uncomment and use the playwright's AxeBuilder
Add tests for settings>roles module
Fix few accessibility errors

Description

(Not ready to merge, because failing tests, before vue-core v2 is also updated with a11y fixes)

Checklist

  • I have added tests to cover this change (if necessary)

Screenshots (optional)

Sorry, something went wrong.

@Tanya-Amber-L Tanya-Amber-L marked this pull request as draft January 30, 2025 09:45
@Tanya-Amber-L Tanya-Amber-L changed the base branch from main to feature/v2 January 30, 2025 09:45
Base automatically changed from feature/v2 to main January 30, 2025 10:59
@@ -129,7 +129,7 @@
--text-tertiary: var(--gray-400);
--text-tertiary-hover: var(--gray-300);
--text-tertiary-on-brand: var(--gray-400);
--text-quaternary: var(--gray-400);
--text-quaternary: var(--gray-500);
Copy link
Contributor

Choose a reason for hiding this comment

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

@wouterlms is het de bedoeling dat deze aangepast wordt van 400 naar 500?

Copy link
Contributor

Choose a reason for hiding this comment

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

Uh, van waar komt deze change @Tanya-Amber-L ?

Copy link
Contributor Author

@Tanya-Amber-L Tanya-Amber-L Jan 30, 2025

Choose a reason for hiding this comment

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

This is for contrast reasons, else the a11y report fails. Needs to stay.
Plus, gray-400 is already used for text-tertiary

Copy link
Contributor

Choose a reason for hiding this comment

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

moet je dat niet de waarde aanpassen van gray-400?

Copy link
Contributor Author

@Tanya-Amber-L Tanya-Amber-L Jan 30, 2025

Choose a reason for hiding this comment

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

Instead of changing every place where gray-400 was used, i decided to change text-quaternary. Because it made ore sense to me rather than keep text-tertiary AND text-quaternary to both point to gray-400

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I copied this from untitled so maybe we need to check if I copied it correctly and if not, fix it indeed. But if it is correct then we need to see where the issue is

@Tanya-Amber-L Tanya-Amber-L marked this pull request as ready for review January 31, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants