Skip to content

fix(media): rewrite media preferences functions #12108

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.

Sign up for GitHub

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

Jump to bottom
Merged
Antreesy merged 3 commits into from
Apr 16, 2024
Merged

fix(media): rewrite media preferences functions #12108

Antreesy merged 3 commits into from
Apr 16, 2024

Conversation

@Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Apr 15, 2024 •
edited
Loading

☑️ Resolves

  • Closes preferred media devices follow-up #12064
  • rewrite media preferences functions
    • fix registering only last device when populate with several
    • fix selecting last available device in list when join (instead of last used | first available)
    • drop 'promote' option as unused (we register promoted device in another place)
    • keep default devices in preferences list
    • remove fallback id logic
    • move console.log from function for testing purposes

🖌️ UI Checklist

🚧 Tasks

  • Check behavior without granted permissions

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖥️ Tested with Desktop client or should not be risky for it
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@Antreesy Antreesy added this to the 💙 Next Major (30) milestone Apr 15, 2024
@Antreesy Antreesy requested a review from DorraJaouad April 15, 2024 08:50
@Antreesy Antreesy self-assigned this Apr 15, 2024
@Antreesy Antreesy force-pushed the fix/12064/follow-up-prefs branch from cd52f2a to 4cc32e9 Compare April 15, 2024 09:25
DorraJaouad
DorraJaouad reviewed Apr 15, 2024
View reviewed changes
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part 1

@Antreesy Antreesy force-pushed the fix/12064/follow-up-prefs branch 4 times, most recently from 8546bc8 to 177180d Compare April 16, 2024 09:49
@Antreesy Antreesy marked this pull request as ready for review April 16, 2024 09:51
@Antreesy Antreesy force-pushed the fix/12064/follow-up-prefs branch from 177180d to ae1da64 Compare April 16, 2024 10:06
@Antreesy Antreesy requested a review from DorraJaouad April 16, 2024 10:07
@nickvergessen
Copy link
Member

nickvergessen commented Apr 16, 2024

There seems to be an issue now with Chromium at least.

After resetting permissions and reloading the page:
grafik

It's only asking for my camera permission after several seconds and even after granting it does not allow to enable the camera.
Browser console is yelling: Error getting video stream: OverconstrainedError:

@nickvergessen
Copy link
Member

nickvergessen commented Apr 16, 2024

If I reload after accepting permissions it seems to be working properly

Antreesy added 3 commits April 16, 2024 12:53
@Antreesy
fix(media): refactor preferences functions
fcc67a3 
- fix registering only last device when populate with several
- fix selecting last available device in list when join (instead of last used | first available)
- move console.log from function for testing purposes
- add types, enum, refactor functions
- drop 'promote' option as unused (we register promoted device in another place)

Signed-off-by: Maksim Sukharev <[email protected]>
@Antreesy
fix(media): keep default devices in preferences list, remove fallback…
84171f7 
… id logic

Signed-off-by: Maksim Sukharev <[email protected]>
@Antreesy
fix(media): add test coverage
168280d 
Signed-off-by: Maksim Sukharev <[email protected]>
@Antreesy Antreesy force-pushed the fix/12064/follow-up-prefs branch from ae1da64 to 168280d Compare April 16, 2024 11:30
DorraJaouad
DorraJaouad approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in different senarios, ✅

When user selects nothing (None) , preferences won't be applied anymore as we expect the user doesn't want to have any device set anymore.

When user manually selects in preview and without joining call, we assume was just experimenting devices and we do not update preferences.

@Antreesy Antreesy mentioned this pull request Apr 16, 2024
Closed
@Antreesy Antreesy merged commit 2d20812 into main Apr 16, 2024
@Antreesy Antreesy deleted the fix/12064/follow-up-prefs branch April 16, 2024 12:17
@nickvergessen
Copy link
Member

nickvergessen commented Apr 16, 2024

/backport to stable29

@backportbot backportbot bot mentioned this pull request Apr 16, 2024
Merged
@danxuliu danxuliu mentioned this pull request Apr 18, 2024
Merged
@danxuliu danxuliu mentioned this pull request Apr 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DorraJaouad DorraJaouad DorraJaouad approved these changes

Assignees

@Antreesy Antreesy

Labels

2. developing feature: call 📹 Voice and video calls feature: frontend 🖌️ "Web UI" client

Projects

None yet

Milestone

v20.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

preferred media devices follow-up

4 participants

@Antreesy @nickvergessen @DorraJaouad