FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Add confirmation popup before unsubscribing

Open msagr opened this issue 1 year ago • 15 comments

Added a popup to ask for confirmation before unsubscribing

Pull Request Type

  • [ ] Bugfix
  • [x] Feature Implementation
  • [ ] Documentation
  • [ ] Other

Related issue

closes #4109

Description

It asks for a confirmation while unsubscribing channel, to avoid accidental unsubscription.

Screenshots

Screenshot from 2024-04-08 01-18-53

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

  • **OS:**Ubuntu
  • **OS Version:**22.04
  • **FreeTube version:**0.20.0

msagr avatar Apr 07 '24 19:04 msagr

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 ft-prompt component so that it matches the popups throughout the rest of the app.

absidue avatar Apr 09 '24 05:04 absidue

I think there should be a toggle for this

PikachuEXE avatar Apr 09 '24 09:04 PikachuEXE

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 avatar Apr 09 '24 13:04 msagr

@msagr yes please implement the suggested changes :)

Thanks @efb4f5ff-1298-471a-8973-3d47447115dc for the review, I will try to implement the suggested changes.

msagr avatar Apr 15 '24 15:04 msagr

@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?

msagr avatar Apr 17 '24 03:04 msagr

popup playing hide and seek on the player page

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?

Screenshot_20240420_082313

kommunarr avatar Apr 20 '24 13:04 kommunarr

Key difference from you screenshot vs mine

Your player is in theatre mode and mine isnt. Did you test this with both modes?

Edit: sorry for the temporary laziness. Yes, it works with both modes:

Screenshot_20240422_170208

kommunarr avatar Apr 22 '24 20:04 kommunarr

Approving after #3975 is merged

Needs to be rebased with development, and then it should be working as intended

kommunarr avatar May 01 '24 01:05 kommunarr

@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)

kommunarr avatar May 01 '24 11:05 kommunarr

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar May 31 '24 01:05 github-actions[bot]

Unfortunately it looks like @msagr is busy, it would be a pity if his cool work was lost.

I don't know about programming but I gather that all the work is done and that bringing it to the current version could be very easy. Would it be impolite to ask if another dev could put the finishing touches on it?

zipfile6209 avatar Jun 08 '24 17:06 zipfile6209

The devs are people with very limited time because we all do this in our spare time. If we had to take over every PR that gets abandoned we would never get to the important stuff.

We have way more important stuff todo with our limited time than addressing low priority issues like this one.

New contributors feel free to take the code of the PR and make the necessary changes