Skip to content

USWDS-Site - Settings: Fix code audit issues #2016

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

Merged
merged 28 commits into from
Mar 20, 2023
Merged

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Feb 27, 2023

Summary

Resolved inconsistencies with what is found in USWDS.

Related issue

Closes #2009

Preview link

Preview link: Settings page

Problem statement

The code documentation audit revealed several inconsistencies with the settings in USWDS 3.3.0.

Individual issues have been itemized in the Code documentation audit spreadsheet (Google sheets) 🔒

Solution

Update settings documentation to accurately reflect what is in the current version USWDS.
⚠️ This does not include any changes included in the forthcoming 3.4.0 release.

Tasks completed

  • Removed settings.yml since it doesn't appear to be used.
  • Repaired settings-table.html to show types in maps
  • Updated settings-table.html to show token types when multiple token types are used
  • Addressed all items discovered in the code documentation audit

Outstanding questions

  • Should$theme-prose-font-family be moved to the components section?
  • Can we remove [token name], [token value] line from $[utility-module]-manual-values?

Testing and review

  • Confirm that all items flagged in the audit have been addressed

Comment on lines +54 to +58
{% for type in setting.type %}
<span class="margin-bottom-05">
{% include get-token-url.html token=type %}
</span>
{% endfor %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows for multiple token types to be declared. Useful for tokens like $theme-collection-header-typeset that need to declare three different tokens in one setting.

Comment on lines -90 to +97
{% include get-token-url.html token=entry.kind %}
{% include get-token-url.html token=entry.type %}
</span>
</td>
<td data-title="{{ col5Title }}" class="{{ col5Width }} display-inline-flex flex-align-start">
<span class="font-lang-3xs">
{{ entry.usage | markdownify }}
{{ entry.description | markdownify }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes the broken token types on mapped items.

@amyleadem amyleadem marked this pull request as ready for review March 7, 2023 23:00
@amyleadem amyleadem requested review from mahoneycm and mejiaj March 7, 2023 23:01
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.

Great findings and work here! Besides being compliant with 3.3.0, I think this also helps our settings to be better understood!

Tested by:

  • Comparing the live site to branch build
  • Cross-referencing audit findings spreadsheet with USWDS settings

@amyleadem amyleadem changed the title USWDS - Settings: Fix code audit issues USWDS-Site - Settings: Fix code audit issues Mar 20, 2023
@thisisdano thisisdano linked an issue Mar 20, 2023 that may be closed by this pull request
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.

Overall looks good, added some comments/questions for potential improvements.

Also found some legacy typo's I was hoping we could fix in this PR. Thank you!

@amyleadem amyleadem merged commit 2e0f34b into main Mar 20, 2023
@amyleadem amyleadem deleted the al-audit-settings-fix branch March 20, 2023 20:43
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 on settings page
4 participants