Skip to content

Conversation

@StaehliJ
Copy link
Contributor

@StaehliJ StaehliJ commented Jul 25, 2024

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

  • Add mobile new settings navigation tab.
  • Add compose overlay to display PlaybackMetrics.

Checklist

  • Your branch has been rebased onto the main branch.
  • APIs have been properly documented (if relevant).
  • The documentation has been updated (if relevant).
  • New unit tests have been written (if relevant).
  • The demo has been updated (if relevant).
  • All pull request status checks pass.

@MGaetan89 MGaetan89 linked an issue Jul 26, 2024 that may be closed by this pull request
3 tasks
Copy link
Contributor

@MGaetan89 MGaetan89 left a 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.

val appSettingsRepository = remember {
AppSettingsRepository(context)
}
val appSettings by appSettingsRepository.getAppSettings().collectAsStateWithLifecycle(AppSettings())
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@MGaetan89 MGaetan89 force-pushed the metrics-collector-ui branch from e32129c to acd574b Compare July 26, 2024 10:22
@github-actions
Copy link

github-actions bot commented Jul 26, 2024

Code Coverage

Overall Project 53.29% -0.01% 🟢
Files changed 84.21% 🟢

Module Coverage
:pillarbox-player 59.26% -0.02% 🟢
Files
Module File Coverage
:pillarbox-player SessionMetrics.kt 88.58% 🟢
QoSCoordinator.kt 85.68% 🟢
BitrateUtil.kt 57.14% -42.86% 🟢

@MGaetan89 MGaetan89 force-pushed the metrics-collector-ui branch from f5a8911 to 7577e27 Compare July 29, 2024 13:05
@StaehliJ StaehliJ merged commit 09eb787 into qos Jul 30, 2024
@StaehliJ StaehliJ deleted the metrics-collector-ui branch July 30, 2024 07:17
MGaetan89 added a commit that referenced this pull request Jul 30, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
MGaetan89 added a commit that referenced this pull request Aug 12, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
MGaetan89 added a commit that referenced this pull request Aug 13, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
MGaetan89 added a commit that referenced this pull request Aug 20, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
StaehliJ added a commit that referenced this pull request Aug 28, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
StaehliJ added a commit that referenced this pull request Sep 11, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
MGaetan89 added a commit that referenced this pull request Sep 12, 2024
Co-authored-by: Gaëtan Muller <m.gaetan89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Basic metrics collector UI

2 participants