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

feat(web): add preference for thumbnail playback on hover #7831

Merged

Conversation

samholton
Copy link
Contributor

@samholton samholton commented Mar 10, 2024

Adds a user preference to disable video playback when hovering on the thumbnail.

I added playbackOnIconHover to enable playback by hovering over the play icon even when preference is disabled. I think this is good functionality, but could still leaves a small target to accidentally start playing the video.

True is default value

Addresses #6283

app settings

@alextran1502
Copy link
Contributor

Hello, thank you for the PR and the continuous contribution <3!

The only thing I am not sure about this PR is whether the option should belong to the Appearance group. I think it should be grouped as "Behavior" of some types. What do you think about this?

@samholton
Copy link
Contributor Author

Hello, thank you for the PR and the continuous contribution <3!

The only thing I am not sure about this PR is whether the option should belong to the Appearance group. I think it should be grouped as "Behavior" of some types. What do you think about this?

Yeah, I was trying to figure out the best place to put it. I ended up here mostly because Display original photos is here. I can create a new Behavior top level group and move Display original photos and Play video thumbnail on hover

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 10, 2024

Maybe change appearance to "App Settings".

IMO all the settings that are just for the web client itself should be grouped together.

@samholton
Copy link
Contributor Author

samholton commented Mar 10, 2024

Maybe change appearance to "App Settings".

IMO all the settings that are just for the web client itself should be grouped together.

How about memories and trash? I like the idea of pulling them all under `App Settings'. There is only a single memories setting for now but could nest that later when more are added.

@samholton
Copy link
Contributor Author

samholton commented Mar 11, 2024

I left memories outside app settings because its an actual user setting vs client side preference

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 11, 2024

I left memories outside app settings because its an actual user setting vs client side preference

Yeah, I think client-side preferences can all be under app settings and things that are persisted to the server can have their own feature section.

@jrasm91 jrasm91 merged commit b33cb5f into immich-app:main Mar 11, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants