[Do not merge] Fixes #2472 - Makes a first pass at validation issues #2473
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@zoepage pinging you on this. Because I probably messed up some of the code/template decision you made. Sorry! If there are better ways to fix I'm all hears. |
|
No issue. :) It's not an emergency. |
|
+1, I was talking about some of the redundant SVG text with Patrick Lauke the other week -- I forgot to file a bug on it. |
zoepage
approved these changes
Jun 6, 2018
| @@ -73,7 +73,7 @@ <h1 class="headline-1">Report Site Issue</h1> | |||
| </div> | |||
| <div class="form-radio form-element js-Form-group"> | |||
| <div class="js-Form-information form-label-message"> | |||
| {{ form.browser_test.label(class_='form-label') }} | |||
| <div class="form-label">Did you test in another browser?</div> | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| @@ -26,7 +26,7 @@ <h1 class="headline-1">Report Site Issue</h1> | |||
| </div> | |||
| <div class="form-radio form-element js-Form-group"> | |||
| <div class="js-Form-information form-label-message"> | |||
| <label class="form-label form-required" for="problem_category">Type of bug</label> | |||
| <label class="form-label form-required">Type of bug</label> | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| @@ -3,7 +3,7 @@ | |||
| {% block body %} | |||
| <div> | |||
| {% include "nav/nav.html" %} | |||
| <main role="main"> | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Closing per #2484 (comment). If there's stuff not covered someone feels strongly about, please file a new issue. |
miketaylr
pushed a commit
that referenced
this pull request
Jun 6, 2018
…y `role="button"`, account dropdown trigger) (#2484) * Fix redundant/incorrect SVG alternative text, remove unnecessary role="button" - `aria-role="hide"` is invalid/non-existent markup - if a control already has text, it's redundant to have `<title>` and extra text for an SVG - just set it to `aria-hidden="true"` and remove any text in it - `aria-label` on the focusable control is generally easier/better if providing a name/label to a control, rather than `<title>` inside the SVG, in cases no other text content is present - *never* ever `aria-hidden="true"` a focusable element (as was the case with the "Close search" button) - having `role="button"` on a `<button>` is unnecessary * Fix the account dropdown trigger - `aria-expanded` is probably more appropriate than `aria-pressed` - make sure the aria attribute is changed on the actual `<button>`, not on the `<li>` (this could likely also be fixed by changing the `var navDropDown = ...` but I wasn't sure if there were perhaps other dependencies on that being the `<li>` * Trivial indentation fix (missed this in the previous commit) * Modify test rendering unit test to reflect the changes made to the SVG alternative text
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes issue #2472
Proposed PR background
This just intends to fix some markup and validation issues.
I was investigating something else which was not working well. And it was getting into the way.
It seems unrelated, but the HTML code could benefit of a bit of a clean up.
Anyone can take over this. This is not mine. :)