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

🎨 [#1617] Fixing labels for correct HTML syntax #716

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

jiromaykin
Copy link
Contributor

@jiromaykin jiromaykin commented Jul 25, 2023

Labels should not be surrounding other labels = incorrect HTML that also makes some accessibility tests fail.
So this PR attempts to remove unnecessary Label tags while keeping the styling the same.
Also any styling that targets the Label TAG instead of the .label class needs to be either removed or needs the .label class added to it.
CSS Styling for third-party Label tags has not been changed.
explanation: https://taiga.maykinmedia.nl/project/open-inwoner/task/1617

(PS: removed the 'Label' file because that one was React only).

extra info: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/label
note "Don't place interactive elements such as anchors or buttons inside a label. Doing so makes it difficult for people to activate the form input associated with the label."

@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Merging #716 (c9636d9) into develop (c494770) will not change coverage.
The diff coverage is n/a.

❗ Current head c9636d9 differs from pull request most recent head c6ce805. Consider uploading reports for the commit c6ce805 to get more accurate results

@@           Coverage Diff            @@
##           develop     #716   +/-   ##
========================================
  Coverage    96.24%   96.24%           
========================================
  Files          669      669           
  Lines        23856    23856           
========================================
  Hits         22961    22961           
  Misses         895      895           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jiromaykin jiromaykin force-pushed the fix/1617-fix-labels-html-syntax branch 2 times, most recently from 4dc56c9 to 0fcf59b Compare July 27, 2023 10:10
@jiromaykin jiromaykin marked this pull request as ready for review July 27, 2023 10:12
@jiromaykin jiromaykin requested a review from pi-sigma July 27, 2023 11:40
Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

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

What about search.html 69ff.? Are these nested labels appropriate there?

@jiromaykin jiromaykin added the Accessibility Improving accessibility label Aug 1, 2023
@jiromaykin
Copy link
Contributor Author

What about search.html 69ff.? Are these nested labels appropriate there?

@pi-sigma those I already fixed in the older PR you still have to review :-D (this one: #709) That's the older PR I worked on first - if that one is okay, then those changes should be merged first and will also improve this new one.

@jiromaykin jiromaykin force-pushed the fix/1617-fix-labels-html-syntax branch from 0fcf59b to ae1e5ac Compare August 17, 2023 09:38
@jiromaykin jiromaykin force-pushed the fix/1617-fix-labels-html-syntax branch from c9636d9 to c6ce805 Compare August 17, 2023 15:09
@jiromaykin jiromaykin requested a review from pi-sigma August 17, 2023 15:20
@alextreme alextreme merged commit ae69cd9 into develop Aug 21, 2023
@alextreme alextreme deleted the fix/1617-fix-labels-html-syntax branch August 21, 2023 08:41
pi-sigma pushed a commit that referenced this pull request Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Improving accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants