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
Add confirmation popup before unsubscribing #4896
base: development
Are you sure you want to change the base?
Conversation
src/renderer/components/ft-subscribe-button/ft-subscribe-button.js
Outdated
Show resolved
Hide resolved
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
I like the way you implemented it but im not sure if the issue has valid reasoning for this to exist. Now if you need to unsubscribe one channel from lets say 3 profiles you are encountering this 3 times. Do the same routine for 10 more channels and this will get very annoying very quickly. If some of the other reviewers think this is a valid PR than there should be a toggle for this confirmation in the settings (maybe parental control idk) |
If we do decide to keep this, then it should be changed to use the |
I think there should be a toggle for this |
Yeah, I think that it makes more sense to have a separate option in setting like avoid accidental unsubscription which when enabled, then only the user will be shown popup while unsubscribing. Should I try to implement it this way? |
@msagr yes please implement the suggested changes :) |
Head branch was pushed to by a user without write access
Screencast.from.13-04-24.01.48.16.PM.IST.webm |
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The settings is turned off. Unsub from a channel -> see that the channel is also being unsubbed from the All Channels profile
VirtualBoxVM_Gx5seWOIPL.mp4
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropdown should not close after i removed the channel from a profile
VirtualBoxVM_7ZveIUYxNN.mp4
popup playing hide and seek on the player page
VirtualBoxVM_v5CRDh3dEI.mp4
Thanks @efb4f5ff-1298-471a-8973-3d47447115dc for the review, I will try to implement the suggested changes. |
@efb4f5ff-1298-471a-8973-3d47447115dc, I was trying to move the side videopanel out of focus when the prompt pops up but i am facing difficulty in doing that. Can you please suggest me something? |
I've tested merging this with #3975 and am no longer seeing this bug here. Do we want to wait on this PR until that one is merged? |
Key difference from you screenshot vs mine Your player is in theatre mode and mine isnt. Did you test this with both modes? |
Approving after #3975 is merged |
Needs to be rebased with |
@jasonhenriquez should #4374 be merged first? |
It's arguable, so maybe yes. It's the least destructive action we could have, and easily reversible at that, but should still probably be marked properly given the use case it's trying to solve for (accidental unsubscribing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both PR's are merged. Rebase into development branch
Added a popup to ask for confirmation before unsubscribing
Pull Request Type
Related issue
closes #4109
Description
It asks for a confirmation while unsubscribing channel, to avoid accidental unsubscription.
Screenshots
Testing
I tested it by first subscribing various channels, and everytime I unsubscribed a channel, it would ask me for a confirmation that if I really want to unsubscribe.
Desktop