-
Notifications
You must be signed in to change notification settings - Fork 87
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
feat: Add compositional Banner components #2184
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for updating this and the little refactoring for MediaBlockBody
const getCopy = (language: Language, tld: TLD): GovBannerCopy => { | ||
const lock = <BannerLockImage title="Lock" description="A locked padlock" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: I'm wondering if we need to think about internationalization support differently as we add more languages. This isn't something I'm thinking for this PR or anything just starting to wonder as I review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strongly agreed. Added a i18n label to capture what issues have occurred so far related to this topic
i18n
Summary
Allow consumers to build custom Banners (say if a consumer needed a banner for a state government website)
This change is inspired by the DOL project which is not open source so I cannot link it here, but in short, we needed a custom Banner which GovBanner didn't satisfy the needs of.
I made the choice to leave the existing GovBanner in place because it's a nice convenience for consumers that want the default banner, and there's no easy way to introduce the breaking change workaround of removing it and then telling consumers to build a lengthy compositional component (see the example) just to replace what they had. This way, there's no breaking changes, consumers are happy, and they have more options to boot.
Related Issues or PRs
closes: #2183
closes: #1836
How To Test
GovBanner
has not changedComponents/Banner/Custom Banner
storyScreenshots:
Custom banner storybook example (this one just changes some of the header text, but you can change anything! On the DOL project, we use the compositional structure to also allow us to internationalize all of the text and aria content with i18n!: