Skip to content

refactor: Creates a shared Banner styles file#849

Merged
curlyfriesplease merged 3 commits intomasterfrom
ENG-4433-email-banner-qa-followup
Mar 5, 2026
Merged

refactor: Creates a shared Banner styles file#849
curlyfriesplease merged 3 commits intomasterfrom
ENG-4433-email-banner-qa-followup

Conversation

@curlyfriesplease
Copy link
Contributor

@curlyfriesplease curlyfriesplease commented Mar 5, 2026

PR description

What is it doing?

Tidyup of my Email & Donate banner work: They have a huge amount of duplicated styling code between them.
It's important to have them reference shared styles or inevitably they'll stop mirroring each other as things change.
HeroBanner's styles has some similarities in places, but no overlaps enough to pull out of that too.

Why is this required?

Refactor, code tidyup

link to Jira ticket:

ENG-4433

Quick Checklist:

  • My PR title follows the Conventional Commit spec.

  • I have filled out the PR description as per the template above.

  • I have added tests to cover new or changed behaviour.

  • I have updated any relevant documentation.

Important! - lastly, make sure to squash merge...

@curlyfriesplease curlyfriesplease changed the title Creates a shared Banner styles file, as Donate and Email banners have so much duplicated style code refactor: Creates a shared Banner styles file Mar 5, 2026
return (
<FormWrapper
donateOrientation={donateOrientation}
shouldShowTitleSection={shouldShowTitleSection}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Realised I didn't need this line, styles don't use it

@curlyfriesplease curlyfriesplease marked this pull request as ready for review March 5, 2026 12:19
@curlyfriesplease curlyfriesplease self-assigned this Mar 5, 2026
<div
aria-live="polite"
className="c2"
orientation="right"
Copy link
Contributor

@AndyEPhipps AndyEPhipps Mar 5, 2026

Choose a reason for hiding this comment

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

Whoops, these need to be $transientProps; I always forget about em too

I guess the previous camelCasing of the longer name marks them as transient automatically, hence this only showing up now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good shout. And a good theory about why.
Pushing a fix momentarily

@curlyfriesplease curlyfriesplease merged commit 62fd1a4 into master Mar 5, 2026
9 checks passed
@curlyfriesplease curlyfriesplease deleted the ENG-4433-email-banner-qa-followup branch March 5, 2026 14:41
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

🎉 This PR is included in version 8.62.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants