FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Improves quick bookmark UX

Open PikachuEXE opened this issue 1 year ago • 5 comments

Pull Request Type

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

Related issue

Closes https://github.com/FreeTubeApp/FreeTube/issues/5051

Description

Look at commits in case this list becomes outdated

  • Update quick bookmark icons (changes from https://github.com/FreeTubeApp/FreeTube/pull/5058)
  • Disable quick bookmark now requires a confirmation
  • Prompt user to select a new target when quick bookmark playlist target deleted ~~Disable deleting quick bookmark playlist target (Prevent >1 dangerous action performed with one user action)~~
  • Update user playlists view to allow updating/disabling quick bookmark & see current target
  • Allow user to attempt quick bookmark without target set (new window to save progress, maybe should be same window? no idea)

Screenshots

Testing

Update quick bookmark icons image image

Disable quick bookmark now requires a confirmation image

Prompt user to select a new target when quick bookmark playlist target deleted image

~~Disable deleting quick bookmark playlist target~~ image

Update user playlists view to allow updating/disabling quick bookmark & see current target image

Allow user to attempt quick bookmark without target set image

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

PikachuEXE avatar May 07 '24 01:05 PikachuEXE

Aside from the addition of a deletion prompt, I contend that this is a worse UX than #5058 for the reasons I laid out in our discussion on that PR. I will let the other developers read both threads and speak to make the determination because I just don't think we see eye-to-eye enough on UX design for our dialogue to continue to be productive past a certain point (as evidenced by you creating this PR rather than continuing the discussion where it left off, such that you seem to be inviting this scenario).

kommunarr avatar May 07 '24 02:05 kommunarr

  • Disable quick bookmark now requires a confirmation + Allow user to attempt quick bookmark without target set: I never like forcing user to set a target (and potentially making mistakes without knowing until too late) There is no prompt implemented to select (instead it redirect to user playlists page) as this is what I have written in just an hour for preview

  • Disable deleting quick bookmark playlist target: if the select bookmark target prompt implemented this would be reverted

  • Update user playlists view to allow updating/disabling quick bookmark & see current target: applicable even https://github.com/FreeTubeApp/FreeTube/pull/5058 is picked over this PR, but necessary part of this PR

  • Allow user to attempt quick bookmark without target set: Again since there is no prompt implemented yet this is for preview

I don't consider this as complete yet but at the same time without the prompt in #5058 I don't consider that as complete solution as well

PikachuEXE avatar May 07 '24 03:05 PikachuEXE

Prompt added Description & screenshots updated

PikachuEXE avatar May 08 '24 02:05 PikachuEXE

Just to be clear with this comment I'm not taking sides on which approach is better.

If you really want the prompt, please find a way to implement it that doesn't involve duplicating 90% of the add to playlist prompt code, e.g. with a shared component or even using the same prompt for both.

absidue avatar May 08 '24 06:05 absidue

@absidue Done Not much experience in making shared component let me know what's wrong/left

PikachuEXE avatar May 08 '24 07:05 PikachuEXE

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar May 20 '24 09:05 github-actions[bot]

@PikachuEXE this can be closed now right?