Jr/soi recaptcha newsleter #14443
Jr/soi recaptcha newsleter #14443
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. |
github-actions
bot
removed
the
run_chromatic
| <div css={inputAndOptInWrapperStyle}> | ||
| <span css={inputWrapperStyle}> | ||
| <TextInput | ||
| label="Enter your email" | ||
| value={email} | ||
| onChange={handleTextInput} | ||
| error={errorMessage} | ||
| disabled={status === 'Loading'} | ||
| onFocus={() => | ||
| setFirstInteractionOccurred(true) | ||
| } | ||
| /> | ||
| </span> | ||
| {isMarketingOptInVisible && ( | ||
| <div> | ||
| <CheckboxGroup | ||
| name="marketing-preferences" | ||
| label="Marketing preferences" | ||
| hideLabel={true} | ||
| cssOverrides={optInCheckboxTextSmall} | ||
| > | ||
| <Checkbox | ||
| label="Get updates about our journalism and ways to support and enjoy our work." | ||
| value="marketing-opt-in" | ||
| checked={marketingOptIn} | ||
| onChange={(e) => | ||
| setMarketingOptIn( | ||
| e.target.checked, | ||
| ) | ||
| } | ||
| theme={{ | ||
| fillUnselected: | ||
| palette.neutral[100], | ||
| borderUnselected: | ||
| palette.neutral[46], | ||
| fillSelected: | ||
| palette.brand[500], | ||
| borderSelected: | ||
| palette.brand[500], | ||
| borderHover: palette.brand[500], | ||
| }} | ||
| /> | ||
| </CheckboxGroup> | ||
| </div> | ||
| )} | ||
| {useReCaptcha && !!captchaSiteKey && ( | ||
| <div | ||
| css={css` | ||
| .grecaptcha-badge { | ||
| visibility: hidden; | ||
| } | ||
| `} | ||
| > | ||
| {(!visibleRecaptcha || | ||
| firstInteractionOccurred) && ( | ||
| <ReactGoogleRecaptcha | ||
| sitekey={captchaSiteKey} | ||
| ref={reCaptchaRef} | ||
| onError={handleCaptchaError} | ||
| size={ | ||
| visibleRecaptcha | ||
| ? 'normal' | ||
| : 'invisible' | ||
| } | ||
| /> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
There is a lot of conditional logic in this component we could improve readability by creating a FormFields component and isolate the logic for handling the form inputs, the opt-in and the recaptcha
There was a problem hiding this comment.
Thank you for the review and the nice suggestion! I've implemented this change now
| e.target.checked, | ||
| ) | ||
| } | ||
| theme={{ |
There was a problem hiding this comment.
It looks like you only need to set fillUnselected here? (Which has a default value of transparent.) The other properties are the same as the default values so aren't needed.
There was a problem hiding this comment.
Oh nice, thank you for catching this! I've implemented this change now
github-actions
bot
removed
the
run_chromatic
|
|
||
| const optInCheckboxTextSmall = css` | ||
| label > div { | ||
| font-size: 13px; |
There was a problem hiding this comment.
Has a designer specified the font size to use here? 13px is a non-standard size so it may be better to use one of the typography presets, perhaps textSans14?
https://guardian.github.io/storybooks/?path=/docs/source_foundations-typography--docs
Overriding the label's font size also affects the alignment with the checkbox so you may need to adjust the margin to keep the first line of the label aligned with the centre of the checkbox.
There was a problem hiding this comment.
Thank you for catching this! I updated the size to use 14 which is what is defined in the designs.
What does this change?
Implements the visible reCAPTCHA on the All Newsletters page.
Adds an opt-in checkbox for “similar Guardian products”:
-- Checkbox is not displayed.
-- No marketing param is sent to the backend.
-- Existing subscription status remains unaffected.
-- Checkbox is displayed.
-- The marketing param is sent to the backend with the correct value (true/false) depending on opt-in choice.
Why?
Screenshots
Product note: Product decided not to AB test the checkbox itself. The AB test will apply only to the visible reCAPTCHA, which will be launched in the next PR