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

USWDS-Site: Fix component pages from audit (Phase 2) #2162

Merged
merged 48 commits into from Nov 20, 2023

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Jul 5, 2023

Summary

Updated component pages according to the findings reported in the code/developer content audit (Google Docs 🔒)

To make the review process more manageable, the changes will be implemented in phases.

⚠️ Before merge, we should update all NNNN-NN-NN changelog dates.

Related issue

Closes #2151

Preview link

Solution

Update code according to the findings in code/developer content audit (Google Docs 🔒).

Please note that all items highlighted yellow in the spreadsheet will be investigated/discussed in the last phase of the component page fixes.

Testing and review

  • Confirm that all updates are accurate and make sense
  • Confirm that all meaningful updates have a related changelog
    • "Meaningful" updates here mostly applied to adding or removing documentation about variants, dependencies, etc. Copy tweaks to add clarity were not considered meaningful, but a case could be made for them to be included.
  • Confirm grammar and spelling for all updates

@amyleadem amyleadem changed the title USWDS-Site: Fix component pages from audit (Phase 1) USWDS-Site: Fix component pages from audit (Phase 2) Jul 5, 2023
@@ -1,5 +1,5 @@
- **Initialization properties.** JavaScript will create most elements for file input. To get a file input to initialize, add the class name `usa-file-input` to `<input type="file" />`.
- **Interaction.** When a user selects or drags documents to the file input, the file name and a thumbnail preview are listed.
- **Using the `accept` attribute.** You can allow certain files by placing an accept attribute on the `<input/>`. If a file type is not accepted, the file will not be attached and the file input will display a message. [Learn more about the accept attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#accept) [mozilla.org].
- **Internet Explorer/Edge.** These browsers do not support dragging items to a file input. Instructions to drag files are removed for these browsers.
- **Internet Explorer and older versions of Edge.** These browsers do not support dragging items to a file input. Instructions to drag files are removed for these browsers.
Copy link
Contributor Author

@amyleadem amyleadem Jul 5, 2023

Choose a reason for hiding this comment

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

Is this note still needed? Wondering if it can be considered outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

In practice, does the file-input component remove the drag instructions if implemented on these browsers?

If it's still a feature of the component, I feel we should leave it.

@@ -1,3 +1,3 @@
- **Allow multiple file formats.** Not everyone has access to the same software. Be flexible with file types to avoid unnecessary software requirements.
- **Prefer one file per input.** Some users might not know how to select multiple files in a file browser. Additionally, iOS does not allow multiple-file selection using the Files app.
Copy link
Contributor Author

@amyleadem amyleadem Jul 5, 2023

Choose a reason for hiding this comment

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

Note
Removed the iOS note because I was able to select multiple files and images in iOS.

Comment on lines -33 to -44
<li>
<strong>Use the gov banner on all federal websites.</strong> We recommend
that all federal government websites include the “official government website”
banner and a logo or site name.
</li>
Copy link
Contributor Author

@amyleadem amyleadem Jul 5, 2023

Choose a reason for hiding this comment

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

Note
I removed notes about the banner component on the header page because the content seemed outdated and I worried that mixing component guidance might lead to confusion.

If we want to keep something here - maybe it would be good to convert some of this content to a site-note that recommends using both the banner and logo/site name on your page?

Comment on lines -114 to -125
The Design System uses semantic header and nav elements with
<code>role="banner"</code> and <code>role="navigation"</code> respectively.
<code>role="banner"</code> is your masthead.
</li>
<li>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note
Removed this because we do not use either roles in our banner or header component examples.

@amyleadem amyleadem requested a review from sarah-sch July 5, 2023 22:26
Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

Confirming the relevant changes from the audit sheet have been made and free from spelling or grammatical errors!

I left a few comments weighing in on some of the pending items. Let me know if I can be of any more assistance if you want to divvy up any research items!

@@ -1,5 +1,5 @@
- **Initialization properties.** JavaScript will create most elements for file input. To get a file input to initialize, add the class name `usa-file-input` to `<input type="file" />`.
- **Interaction.** When a user selects or drags documents to the file input, the file name and a thumbnail preview are listed.
- **Using the `accept` attribute.** You can allow certain files by placing an accept attribute on the `<input/>`. If a file type is not accepted, the file will not be attached and the file input will display a message. [Learn more about the accept attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#accept) [mozilla.org].
- **Internet Explorer/Edge.** These browsers do not support dragging items to a file input. Instructions to drag files are removed for these browsers.
- **Internet Explorer and older versions of Edge.** These browsers do not support dragging items to a file input. Instructions to drag files are removed for these browsers.
Copy link
Contributor

Choose a reason for hiding this comment

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

In practice, does the file-input component remove the drag instructions if implemented on these browsers?

If it's still a feature of the component, I feel we should leave it.

Comment on lines 89 to 92
<strong>Add a skip navigation link before the header.</strong> Include skip navigation links to allow users with
screen readers to bypass long navigation lists. Make sure you include an <code>id</code> at the beginning of your
main content and that it matches the skip navigation link. Find more <a
href="http://webaim.org/techniques/skipnav/">information on these links</a> at WebAIM.
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this idea! Do we have any usa-skipnav documentation? I can't seem to find any.

>http://csswizardry.com/2010/10/your-logo-is-an-image-not-a-h1/.</a
>
<a
href="http://csswizardry.com/2010/10/your-logo-is-an-image-not-a-h1/">http://csswizardry.com/2010/10/your-logo-is-an-image-not-a-h1/.</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

This article isn't much newer, but I like the reasoning here:

The first heading on your site should be the one that best describes the content of your page. Sometimes this is your logo–but most of the time it’s not. 1

Footnotes

  1. https://www.fastcompany.com/3016894/should-your-tag-be-your-logo#:~:text=The%20first%20heading%20on%20your,organize%20information%20and%20orient%20users.

_components/icon-list/icon-list.md Outdated Show resolved Hide resolved
Revising for conciseness. We could use "communicate" instead of "explain" if you like that better.
Adding "skip navigation" to make link text more descriptive
editing for conciseness
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Added a few comments, otherwise LGTM.

</li>
<li>
If you’re using a logo that’s text, use an <code>em</code>, not an
<strong>Don't use an H1 for your logo.</strong> If you’re using a logo that’s text, use an <code>em</code>, not an
Copy link
Contributor

Choose a reason for hiding this comment

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

Reflects what's shown in example:
image

Default header preview

>http://csswizardry.com/2010/10/your-logo-is-an-image-not-a-h1/.</a
>
<a
href="http://csswizardry.com/2010/10/your-logo-is-an-image-not-a-h1/">http://csswizardry.com/2010/10/your-logo-is-an-image-not-a-h1/.</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm included to keep it because:

  • Link still works
  • Site is a good resource
  • Guidance reflects what's actually shown in USWDS header

href="https://www.usertesting.com/blog/site-navigation-tree-testing"
>https://www.usertesting.com/blog/site-navigation-tree-testing</a
>
href="https://help.usertesting.com/hc/en-us/articles/115003372331-What-is-Tree-Testing">https://help.usertesting.com/hc/en-us/articles/115003372331-What-is-Tree-Testing</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep for now. Created a new issue to follow-up on this #2187.

_components/modal/guidance/implementation.md Outdated Show resolved Hide resolved
@amyleadem
Copy link
Contributor Author

@mejiaj and @mahoneycm I believe I addressed all your concerns. Let me know if I missed anything.

Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks Amy

@mejiaj
Copy link
Contributor

mejiaj commented Oct 12, 2023

@amyleadem can you fix conflicts? Otherwise looks good to merge.

@mejiaj mejiaj merged commit a166f65 into main Nov 20, 2023
11 checks passed
@mejiaj mejiaj deleted the al-component-audit-fixes-2 branch November 20, 2023 19:18
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.

USWDS-Site - Code audit: Fix inaccuracies in component pages (Phase 2)
4 participants