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

Manage email subscription from user profile #3886

Closed
7 tasks
wa0x6e opened this issue May 8, 2023 · 31 comments
Closed
7 tasks

Manage email subscription from user profile #3886

wa0x6e opened this issue May 8, 2023 · 31 comments
Assignees
Labels
stale Issues that are old and soon to be closed

Comments

@wa0x6e
Copy link
Collaborator

wa0x6e commented May 8, 2023

Issues

Allow users to manage their email preferences directly from snapshot UI

Changes

This will be a new modal, containing a form with the email notifications settings.

If user is not subscribed yet, the 'SUBSCRIBE' link from the header dropdown menu will open a modal with the subscription form (implemented via #3754)
If user is already subscribed, the modal will show the management subscription form

Modal will looks like:


Email subscriptions [x]

[] Proposal creation (<- checkbox)
[] Proposal closing
[] Weekly report

Select all | Unselect all (<- js event to select all checkboxes)

[UPDATE SUBSCRIPTION] (<- submit button)


On submit, we will ask the user to sign a message to prove he's the wallet owner.

How to test

  1. Log in to your account
  2. Open account dropdown menu in header
  3. Click on 'Subscribe'
  4. In the modal, edit the form
  5. Submit the form
  6. Sign the message
  7. See error/success message

To-Do

  • Show the form in relevant places
  • Add modal/message when subscription is successful
  • Add error message on error

Self-review checklist

  • I have performed a full self-review of my changes
  • I have tested my changes on a preview deployment
  • I have tested my changes on a custom domain
  • I have run end-to-end tests yarn cypress:test:e2e, and they have passed

Todo

Since we're now opening 2 different modals with the same link, maybe we should rename this menu 'Email notifications'

@wa0x6e wa0x6e assigned wa0x6e and Todmy and unassigned wa0x6e May 8, 2023
@wa0x6e wa0x6e changed the title feat: manage email subscription from user profile Manage email subscription from user profile May 8, 2023
@samuveth
Copy link
Contributor

samuveth commented May 9, 2023

Can you clarify whether the subscription settings are stored within the user profile or the envelope? Furthermore, is there an existing method to set and retrieve these settings?

@samuveth
Copy link
Contributor

samuveth commented May 9, 2023

@Todmy @wa0x6e , I noticed that the designs for subscription management haven't been included in the project pitch. Could you please provide some initial design or wireframes for us to review and agree upon before we proceed with the coding implementation?

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 9, 2023

User subscriptions are stored with envelop.
List of different subscriptions options are stored on envelop.

Both data can be retrieved with an api request to envelop.

There will be a rest api to update the subscriptions.

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 9, 2023

First post has been updated with the new specs

@Todmy
Copy link
Contributor

Todmy commented May 12, 2023

@samuveth what do you think about this?:

Image

Let's start with this, and we can decide how it will look.

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 12, 2023

What I am visualizing:

  • Check all should be a link
  • Button should says: "Update preferences"

Are those the default checkbox look&feel from the UI chart ? these looks so much like radio buttons, rather than checkboxes

@bonustrack
Copy link
Member

We should always use "Sentence case" not "Title Case"

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 12, 2023

maybe also add "Uncheck all"

@samuveth
Copy link
Contributor

samuveth commented May 13, 2023

Yes, these are our checkboxes although the design for them isn't set in stone yet. We could alternatively use TuneSwitch component instead of TuneCheckbox
image

We don't need "Uncheck all" in this case

@Todmy
Copy link
Contributor

Todmy commented May 13, 2023

Image

I have tried to consider all your ideas. What do you think?

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 13, 2023

@samuveth Why do you think we don't need the check/uncheck all ?

It make sense only if the check/uncheck event is also triggering a save, which is not the case here

@samuveth
Copy link
Contributor

samuveth commented May 14, 2023

We should consider incorporating a dynamic button, as suggested in your design found here: snapshot-labs/envelop#55 (comment)

This button's functionality would depend on the current state of the items. If more than one item is selected, the button should display "Uncheck All". And if less than two items are selected, the button should change to "Check All". WDYT?

edit: Would have been great to have that design in this issue from the beginning. @wa0x6e

@samuveth
Copy link
Contributor

@Todmy can you also add the sub text from the design I linked about?

"Selected the email categories you wish to receive:"

And also the sub text for each item too

@Todmy
Copy link
Contributor

Todmy commented May 14, 2023

Updated

Image

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 14, 2023

We should follow the list order returned by /subscriptionsList, to ensure everyone have the same order, instead of having the subscribed items first

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 14, 2023

If putting the check all button at this place, it should be a submit button, and the label should reflect that by saying "Subscribe to all".

If it's purpose if just to check the checkboxes, without submitting the form, it should not look like a submit button

@Todmy
Copy link
Contributor

Todmy commented May 15, 2023

@wa0x6e it's just design. During implementation, I will use the list provided by BE.

@Todmy
Copy link
Contributor

Todmy commented May 15, 2023

Image

I have updated the second button text.

Also, I would like to know how you want it to behave. When half or more than half of selected, then it should allow you to subscribe to all types of email, otherwise it will unsubscribe the user from all types of subscriptions. Does it correct?

@samuveth
Copy link
Contributor

samuveth commented May 15, 2023

By the way I think the text should be improved:

Subscription management
Choose the types of email updates that matter to you:

Proposal creation
Get informed when a new proposal is submitted in your followed spaces.

Proposal closure
Get informed when a proposal is closed in your followed spaces.

Weekly summary
Get a weekly report detailing the activity in your followed spaces.

@Todmy
Copy link
Contributor

Todmy commented May 15, 2023

except of that does everything looks good for you?

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 15, 2023

That second button is still bothering me. I think we should just remove it altogether.

It's bad UX to have a submit button do different thing, depending on what is checked.

Either we have

  • Uncheck all | check all (simple link, above the blue submit button)
  • No second button nor links, just the blue one for update

Reproducing the second button from snapshot-labs/envelop#55 (comment) is not ideal, as the page is targeted toward user who already wants to unsubscribe, the page was introduced to try to retain them as last resort.

We should not have a button to facilitate unsubscription from snapshot side, because the emails are removed from DB, and they will be required to subscribe and verify email again. (if user can reactivate emails without all the hassle, it will be a different story).

@Todmy
Copy link
Contributor

Todmy commented May 15, 2023

could you share some examples of how you want to see a link "check all/uncheck all"? Maybe I can see it somewhere on the website? Or maybe you have in mind any other suitable references

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 15, 2023

I was thinking something like that
Screenshot 2023-05-15 at 3 27 35 PM

Let's just agree to remove it altogether, all of the site I've tested does not have this button anymore, and it makes sense only on an unsubscribe page, and not manage preferences page.

@samuveth
Copy link
Contributor

samuveth commented May 15, 2023

I think we should remove the "un/subscribe to all" button. We can handle unsubscriptions from the unsubscribe link in the email.

@Todmy
Copy link
Contributor

Todmy commented May 15, 2023

here is an improvement added to the task. We need to add an ability for a client to remove email from DB completely. This is the draft of design:

Image

this checkbox appears only if all switches are turned off.

@aurelianoB
Copy link
Contributor

I can live with this design for now. Probably worth revisiting once we have a designer. As for the text in the last checkbox, maybe we could just make it a one liner by saying "Remove my email from Snapshot's database".

@samuveth might come up with better text though.

@wa0x6e
Copy link
Collaborator Author

wa0x6e commented May 15, 2023

I propose "Also remove my email from Snapshot's database" as text

Let's also align the text with the rest of the form

@Todmy
Copy link
Contributor

Todmy commented May 15, 2023

updated

Image

@aurelianoB
Copy link
Contributor

Nit, but note that checkbox will only show if all toggles are switched off

@Todmy
Copy link
Contributor

Todmy commented May 16, 2023

@aurelianoB Yes, definitely.

@stale
Copy link

stale bot commented Sep 13, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issues that are old and soon to be closed label Sep 13, 2023
@stale stale bot closed this as completed Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that are old and soon to be closed
Projects
None yet
Development

No branches or pull requests

5 participants