Skip to content

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Nov 15, 2023

Summary

Adds changelog for uswds/uswds#5616

Related PR

uswds/uswds#5616

Preview link

@mahoneycm mahoneycm changed the title USWDS-Site - Changelog: Header compilation error [#5616] USWDS-Site - Changelog: Header compilation error [#5624] Nov 27, 2023
@@ -3,7 +3,7 @@ type: component
changelogURL:
items:
- date: NNNN-NN-NN
summary: Resolved a compilation error when `$theme-max-header-width` is set to non-breakpoint values.
summary: Fixed a bug that prevented `$theme-max-header-width` from accepting a value of "none".
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that the test has expanded to check for non-breakpoint values, should we include that in the changelog?

Copy link
Contributor

Choose a reason for hiding this comment

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

Really, this PR doesn't touch $theme-header-MAX-width other than removing it from the Safari fix entirely. The root issue is that the calculation in the Safari bug fix caused the a value of "none" on the setting o throw an error. So by not using the max-width setting in the Safari fix, that specific bug is removed. I think that should be the focus of the changelog.

Regarding $theme-header-MIN-width, I don't think there is any need to get into the details. Nothing is really different other than the Safari bug won't be applied to values that aren't acceptable to the setting anyway. That piece feels more like a behind-the-scenes thing.

Curious to hear what you think!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amyleadem Oh I think that makes sense. The focus on the existing bug is a good move! 👍

@amyleadem amyleadem requested a review from mejiaj December 8, 2023 19:11
@amyleadem amyleadem changed the base branch from main to release-3.7.1 December 8, 2023 19:33
@amyleadem amyleadem requested a review from thisisdano December 8, 2023 23:32
@thisisdano thisisdano merged commit e8cad8c into release-3.7.1 Dec 8, 2023
@thisisdano thisisdano deleted the cm-changelog-5617 branch December 8, 2023 23:34
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