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

fix: defer email subscription state fetching #4503

Merged
merged 3 commits into from
Jan 14, 2024
Merged

fix: defer email subscription state fetching #4503

merged 3 commits into from
Jan 14, 2024

Conversation

wa0x6e
Copy link
Collaborator

@wa0x6e wa0x6e commented Jan 13, 2024

Summary

A request is sent to the envelop API on page load, to retrieve whether the current user is already subscribed or not. This was made so that when the user click on the "Email subscription" menu, it open the correct modal.

This PR introduces a change, to defer this request to only triggered when the user click on the "Email subscription" menu, thus saving a request on each page load, for user not interested in the email subscription.

This change introduces a change on the UI:

  • Since we don't know the user subscription state when he open "Email subscription" modal, a loading skeleton will be show until we get the state from envelop API, before showing the correct modal content.
  • In the PostVoteModal, we will charge the user subscription status on mounting, so there will be a UI flash, where the "Subscribe to email" button will first not exist, then appear after status fetching
  • Some minor margin adjustment for consistency: use m-4 and gap-4 everywhere

Closes: #4501

How to test

  1. Load any page
  2. It should not trigger any request to https://core.envelop.fyi/subscriber
  3. Connect your wallet
  4. Go to the user menu, and click on "Email subscription"
  5. It should open a placeholder modal, and trigger a request to https://core.envelop.fyi/subscriber
  6. Once the request succeed, it should show the correct modal content, depending on your subscription state
  7. Closing the menu will trigger a request to https://core.envelop.fyi/subscriber, if you're already subscribed, or pending email verification (so next modal opening have correct state)
  8. Reopening the email subscription modal should not trigger any request to https://core.envelop.fyi/subscriber, and should remember the previous state (so no placeholder modal anymore)

@wa0x6e wa0x6e requested a review from samuveth January 13, 2024 05:45
@wa0x6e wa0x6e added the enhancement New feature or request label Jan 13, 2024
Copy link

vercel bot commented Jan 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
snapshot-testnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 13, 2024 3:13pm

Copy link
Contributor

@samuveth samuveth left a comment

Choose a reason for hiding this comment

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

Works well, one thing I noticed is if I enter a wrong email address for verification there is no way to change it and the UI keeps showing the verify your email modal.

tACK

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented Jan 14, 2024

Works well, one thing I noticed is if I enter a wrong email address for verification there is no way to change it and the UI keeps showing the verify your email modal.

tACK

Correct, need to add feature to resend verification email, and to change email

@samuveth samuveth merged commit 79394f2 into master Jan 14, 2024
6 checks passed
@samuveth samuveth deleted the fix-4501 branch January 14, 2024 06:14
@wa0x6e
Copy link
Collaborator Author

wa0x6e commented Jan 15, 2024

Works well, one thing I noticed is if I enter a wrong email address for verification there is no way to change it and the UI keeps showing the verify your email modal.
tACK

Correct, need to add feature to resend verification email, and to change email

In fact, see #3904

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: defer mail subscriber status query
2 participants