14890 show or hide the sign up newsletter component #14929
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. |
…nto 14890-show-or-hide-the-sign-up-newsletter-component
…nto 14890-show-or-hide-the-sign-up-newsletter-component
…nto 14890-show-or-hide-the-sign-up-newsletter-component
| }: EmailSignUpWrapperProps) => { | ||
| const [idApiUrl] = useState(() => { | ||
| if (typeof window === 'undefined') return ''; | ||
| return window.guardian?.config?.page?.idApiUrl ?? ''; |
There was a problem hiding this comment.
Might it be better to return undefined rather than an empty string here and then make the type string | undefined?
…nto 14890-show-or-hide-the-sign-up-newsletter-component
…ttps://github.com/guardian/dotcom-rendering into 14890-show-or-hide-the-sign-up-newsletter-component
…te the type comparision
georgerichmond
left a comment
georgerichmond
left a comment
There was a problem hiding this comment.
Looks good - just wondering about how we best test this - perhaps using the hook in the story is useful rather than repeating the content?
| /** | ||
| * A hook to check if a user is subscribed to a specific newsletter. | ||
| */ | ||
| export const useNewsletterSubscription = ( |
There was a problem hiding this comment.
Is it worth having a test for this?
There was a problem hiding this comment.
It could be tested as part of testing the component by mocking the fetch call possibly
| hidePrivacyMessage?: boolean; | ||
| } | ||
|
|
||
| const AlreadySubscribedWrapper = (props: EmailSignUpWrapperProps) => { |
There was a problem hiding this comment.
Could we use the hook in the react story and just mock out what the service call returns?
There was a problem hiding this comment.
Either by mocking fetch or if there is a standard way it's mocked out in the codebase so that we can see the hook being used in the story.
Otherwise it's just repeating logic from the hook for the point of the story - may as well just show the component as it looks with and without a subscription
|
I couldn't see the story in storybook for some reason |
What does this change?
Investigate hiding the sign up component
Why?
There is an ambition to hide the sign up component that appears in articles when a user has already signed up to the newsletter in question. This would be a nice "quality of life" update that should make the sign up embeds less intrusive.
Ticket link: #14889
Subtask ticket link : #14890
Screenshots