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

Port basket newsletter JS to Protocol (Fixes #839) #905

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

alexgibson
Copy link
Member

@alexgibson alexgibson commented Oct 26, 2023

Description

This PR ports the JS code (and tests) from bedrock that handles newsletter form submissions.

This will help provide Mozilla websites with an up to date example of how to handle newsletter submissions that post directly to Basket, and will act as a replacement for https://github.com/mozilla/basket-example.

I also fixed a couple of other pesky long time bugs, like how small <legend> elements were rendered in Zilla Slab, making them hard to read.

  • I have documented this change in the design system.
  • I have recorded this change in CHANGELOG.md.

Issue

#839

Testing

@alexgibson alexgibson added P2 Second level priority - Should have Review: XS Code review time: 30 mins or less Needs:Review 👋 Ready for Developer Review labels Oct 26, 2023
@reemhamz reemhamz self-assigned this Nov 27, 2023
Copy link
Contributor

@reemhamz reemhamz left a comment

Choose a reason for hiding this comment

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

It works!
I tested it out, and I saw the form submission go to Basket in dev tools.

However, instead of a POST request, I'm getting an OPTIONS request. Is this expected? I'm assuming that's the case because it's determining if the URL it wants to POST to is safe to send?

In any case, it just needs a rebase. r+(wc?) 🍅

@alexgibson
Copy link
Member Author

alexgibson commented Nov 28, 2023

@reemhamz thanks for the review! (rebased)

The way CORS (cross origin) requests work is that browsers will first send an OPTIONS pre-flight request to make sure the site is allowed to make the cross origin request. If allowed, it will then make the POST. You should hopefully see two requests in the network console?

image

@reemhamz
Copy link
Contributor

Ah yup, I see both requests in the console :) so this is good to merge, but I'll leave that to you! 👍🏼

@alexgibson
Copy link
Member Author

Thanks for double checking!

@alexgibson alexgibson merged commit 7e6e9fd into mozilla:main Nov 29, 2023
2 checks passed
@alexgibson alexgibson deleted the newsletter-js branch November 29, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs:Review 👋 Ready for Developer Review P2 Second level priority - Should have Review: XS Code review time: 30 mins or less
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants