Add photo player presentation delay to settings as user preference - #5722
Add photo player presentation delay to settings as user preference#5722hraabis wants to merge 9 commits into
Conversation
…n settings, playback, photo player.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| modifier = Modifier.sizeIn(minWidth = 32.dp), | ||
| contentAlignment = Alignment.CenterEnd | ||
| ) { | ||
| Text("${photoPlayerPresentationDelay / 1000}s") |
There was a problem hiding this comment.
Same implementation of formatting value as seconds is found in SettingsPlaybackAdvancedScreen.kt and SettingsPlaybackNextUpScreen.kt. Should all be updated or this one ignored.
There was a problem hiding this comment.
New timeUtils function to convert millis to seconds added to remove usage of magic number for displaying seconds.
nielsvanvelzen
left a comment
nielsvanvelzen
left a comment
There was a problem hiding this comment.
I don't see much benefit to making this a global preference, we had a PR going that was almost to the finish line and you just closed it...
|
Yes, I was sorry to do so, but closed it when the user interface of the last implementation was not viable and I considered the benefits of using the settings menu. The last PR stored time delay as a system preferences, which I understood to store the value globally for all users, this PR implements it as a user preference. Allowing each user to set a different time delay. The last PR would require users wanting different time delays to switch it each time the other user changed it. Using the settings menu allows for easier addition of new photo player preferences, keeps the photo player controls centred, and provides text for explanations of the preferences. |
Both the UserPreferences and SystemPreferences are globally stored on-device for all the Jellyfin users. They are not specific to a single user.
Just because it's easier doesn't mean it's better
There's a lot of ways to fix that. Like setting a fixed width on the element, moving the text below the icon etc. |
… TimeUtils millisToSeconds function to convert values using constant values. Add TimeUtilsTests.kt for new function.
Sorry, I didn't understand both were stored globally and thought each account would have it's own preferences.
There's benefits to both implementations. I liked the photo player control button for quickly switching delay time when viewing different photo folders. I like the settings menu range slider for allowing any time delay setting between the min and max.
With the routes for settings pages, could a settings button (gear icon) be added to the photo player controls to open the playback, photo player page. It would provide quick access to the relevant settings and keep new features from adding buttons to the UI. Or, implementing it as a popup menu like the PlaybackSpeedAction on video playback controls. |
|
Adding the new Settings page for Photo player playback provides a better place to add settings for the next feature I'm working on, the documentary-style animation with effect strength settings. I already have the code ready as a work in progress branch. |
|
I guess that could work too, having it as a screen that is openable from both the settings and from the photo player directly. We already do a similar thing for the library settings. |
…tings page. Add settingsVisible value to PhotoPlayerViewModel.kt to control settings menu visibility. Restart photo presentation when settings close to apply new settings.
4 participants
Changes
Add photo player presentation delay to settings as a user preference. In Settings, Playback, add new Photo player menu button and new screen. On photo player screen, set presentation delay with range control from 4 seconds to 120 seconds.
Code assistance
No code assistance used.
Issues
Resolve slideshow speed