Skip to content

USWDS - Date picker: Remove empty attrs.#4218

Merged
thisisdano merged 5 commits into
developfrom
seanpclark-date-picker-non-empty-id
Jul 6, 2021
Merged

USWDS - Date picker: Remove empty attrs.#4218
thisisdano merged 5 commits into
developfrom
seanpclark-date-picker-non-empty-id

Conversation

@mejiaj

@mejiaj mejiaj commented Jun 4, 2021

Copy link
Copy Markdown
Contributor

Description

Builds on the work from @seanpclark in #4200. Additionally removes empty name attribute.

Additional information

TK

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

);
internalInputEl.id = "";
internalInputEl.removeAttribute("id");
internalInputEl.removeAttribute("name");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not breaking, but was leaving an empty name attribute. Went ahead and removed it.

ANDI issue: Aria-hidden alerts.

The element is being hidden from screenreaders anyways. Using `sr-only` class here could be potentially confusing.
DATE_PICKER_INTERNAL_INPUT_CLASS
);
internalInputEl.id = "";
internalInputEl.style.display = "none";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the sr-only class and hiding it via display: none instead. This resolves an ANDI issue that was flagged about hidden focusable elements.

Error USWDS 2.11.2
image

@mejiaj

mejiaj commented Jun 7, 2021

Copy link
Copy Markdown
Contributor Author

Other ANDI issues:

usa-date-picker__wrapper was removed from the tab order with tabindex="-1", but it has focusable elements inside. We could probably remove that attribute on the wrapper.

Removed negative tabindex in c76974e.

image


The icon is set via background on the button. We could move that into an inner span like we did on the footer social icon nav in #4211

image

@mejiaj mejiaj marked this pull request as ready for review July 6, 2021 15:45
@thisisdano thisisdano merged commit 8f33279 into develop Jul 6, 2021
@mejiaj mejiaj deleted the seanpclark-date-picker-non-empty-id branch August 12, 2021 18:59
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.

3 participants