-
Notifications
You must be signed in to change notification settings - Fork 30
added feature flag for signup component #15027
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
base: main
Are you sure you want to change the base?
added feature flag for signup component #15027
Conversation
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
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.
Pull request overview
This PR introduces a feature flag (hideNewsletterForSubscribers) to control whether the newsletter subscription check is performed, allowing the newsletter signup component to be shown regardless of a user's subscription status when the flag is disabled.
- Added
enableCheckparameter touseNewsletterSubscriptionhook with a default value oftrue - Connected the feature flag from the frontend config through the component chain
- Added comprehensive test coverage for the new feature flag behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dotcom-rendering/src/lib/useNewsletterSubscription.ts | Added enableCheck parameter to hook that skips API call and returns false when disabled |
| dotcom-rendering/src/lib/useNewsletterSubscription.test.ts | Added test coverage for feature flag enabled, disabled, and default behavior scenarios |
| dotcom-rendering/src/lib/renderElement.tsx | Passes the hideNewsletterForSubscribers switch to the newsletter component |
| dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx | Added story demonstrating the feature flag disabled state |
| dotcom-rendering/src/components/EmailSignUpWrapper.importable.tsx | Added hideNewsletterForSubscribers prop and passes it to the subscription hook |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dotcom-rendering/src/components/EmailSignUpWrapper.importable.tsx
Outdated
Show resolved
Hide resolved
mgosz-guardian
left a comment
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. One question in the comment.
…nto 442-add-feature-flag-toggle-for-sign-up-component
What does this change?
I'm handling the API call through the feature flag, and the feature flag is being passed from the frontend repo via the config.
Why?
We will look to add a feature flag to toggle to identity api call.
Reference PR
Ticket link
Reference Ticket link: #14889
Reference Subtask ticket link : #14890
Screenshots