Skip to content
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(newsletters): Update newsletter checkbox labels, add new newsletter checkbox w/two slugs #15482

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

LZoog
Copy link
Contributor

@LZoog LZoog commented Jun 27, 2023

Because:

  • We want to update the label text for two existing newsletters and add a new option to subscribe users to two new newsletters from one new checkbox

This commit:

  • Updates labels per ticket
  • Adds new checkbox with array of slugs for new newsletter option, does not HTML escape label due to ampersand
  • Updates server validity check, tests

Closes FXA-7527

@LZoog LZoog requested a review from a team June 27, 2023 17:20
@LZoog LZoog requested a review from a team as a code owner June 27, 2023 17:20
slug: 'take-action-for-the-internet',
},
ONLINE_SAFETY: {
label: t('Be safer and smarter online'),
slug: 'knowledge-is-power',
},
};

export const newsletterNewCopy = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this since it wasn't being used anywhere.

I was thinking of removing other unused newsletters like ONLINE_SAFETY and FIREFOX_ACCOUNTS_JOURNEY and updating auth-server's validity check for only these 3, happy to do that; I'd left them as-is since this way we've got a pool of available/valid newsletters and then we pull in the ones we want on the page 🤷‍♀️

…ter checkbox w/two slugs

Because:
* We want to update the label text for two existing newsletters and add a new option to subscribe users to two new newsletter from one new checkbox

This commit:
* Updates labels per ticket
* Adds new checkbox with array of slugs for new newsletter option, does not HTML escape label due to ampersand
* Updates server validity check, tests

Closes FXA-7527
Copy link
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

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

:shipit:

@LZoog LZoog merged commit 59441a9 into main Jun 29, 2023
@LZoog LZoog deleted the FXA-7527 branch June 29, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants