-
Notifications
You must be signed in to change notification settings - Fork 4
Metrics collector UI #655
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
Metrics collector UI #655
Conversation
MGaetan89
left a comment
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.
Just a few comments.
I took the liberty of updating the setting screen, so its design is more in line with the rest of the demo app. It's in a dedicated commit (e32129c), so it's easy to revert if needed.
...mo-shared/src/main/java/ch/srgssr/pillarbox/demo/shared/ui/settings/MetricsOverlayOptions.kt
Show resolved
Hide resolved
| val appSettingsRepository = remember { | ||
| AppSettingsRepository(context) | ||
| } | ||
| val appSettings by appSettingsRepository.getAppSettings().collectAsStateWithLifecycle(AppSettings()) |
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.
Why not use the AppSettingsViewModel here?
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.
We could yes, in won't be short to write and the viewmodel will be scoped to the Activity if I'm not wrong.
pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/ui/player/PlayerView.kt
Show resolved
Hide resolved
pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/MainNavigation.kt
Outdated
Show resolved
Hide resolved
pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/utils/BitrateUtil.kt
Show resolved
Hide resolved
pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/utils/BitrateUtil.kt
Outdated
Show resolved
Hide resolved
e32129c to
acd574b
Compare
Code Coverage
Files
|
8a29727 to
24e94a5
Compare
Also update `CustomPlaybackSettingsShowcase` to have the same behavior
f5a8911 to
7577e27
Compare
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Pull request
Description
The Goal of this PR is to had a metrics overlay into the demo. The overlay can be enabled and customized in the new tab "setting"
Changes made
PlaybackMetrics.Checklist
mainbranch.