Visitar URL original
USWDS - Remove id from site title by amyleadem · Pull Request #5319 · uswds/uswds · GitHub
Skip to content

USWDS - Remove id from site title - #5319

Merged
thisisdano merged 4 commits into
developfrom
al-extended-megamenu-bug-5305
Jun 8, 2023
Merged

thisisdano merged 4 commits into
developfrom
al-extended-megamenu-bug-5305

Conversation

@amyleadem

@amyleadem amyleadem commented Jun 5, 2023 •

Copy link
Copy Markdown
Contributor

Summary

  • Fixed a bug that prevented the header from finding the designated site_title content during the HTML build process. The site title appeared with the accurate value in Storybook, but showed the default Home value in the html files.
  • Removed the id attribute from the usa-logo element. This id required maintenance of multiple lines of JSON but did not seem to serve a clear purpose.
    • This might be too bold of an approach. If we prefer, we can keep the id attribute and just create a standard value for all uses.

Related PR

This was originally intended to be merged into #5311.

Preview link

Related PR

Changelog PR: uswds/uswds-site#2124

Testing and review

  • Confirm that the site title content in the header component is <Project title> for both Storybook and the following files in html-templates:
    • usa-header.html
    • usa-header~megamenu.html
    • usa-header--extended.html
    • usa-header--extended-megamenu.html
  • Consider if there is any need for the id in the site title and if it is advisable to remove.

@amyleadem
amyleadem marked this pull request as ready for review June 6, 2023 14:17
@amyleadem
amyleadem requested review from mahoneycm and mejiaj June 6, 2023 14:24

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, I tested:

  • Output HTML from npm run build:html
  • StorybookJS components (headers, site title, and nav partial).

@mahoneycm mahoneycm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

  • Confirmed ID has been removed
  • Confirmed correct title is displaying in storybook as well as html templates after build:html
  • Confirmed USWDS is not referencing the ID in any existing code.

Base automatically changed from jm-extended-megamenu-bug-5305 to develop June 6, 2023 22:42
@amyleadem amyleadem added this to the uswds 3.5.0 milestone Jun 7, 2023
@amyleadem

amyleadem commented Jun 7, 2023 •

Copy link
Copy Markdown
Contributor Author

@thisisdano This PR was originally intended to be merged into #5311 so it does not have its own issue. #5311 has already been merged into develop, so I've tagged this as a separate 3.5.0 item.

@thisisdano
thisisdano merged commit cf8c634 into develop Jun 8, 2023
@thisisdano
thisisdano deleted the al-extended-megamenu-bug-5305 branch June 8, 2023 17:59
@thisisdano thisisdano mentioned this pull request Jun 9, 2023
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.

4 participants