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.

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

Allow join unmuted in widget mode #2848

Open
wants to merge 13 commits into
base: livekit
Choose a base branch
from
Open

Allow join unmuted in widget mode #2848

wants to merge 13 commits into from

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Nov 28, 2024

No description provided.

@toger5 toger5 requested a review from a team as a code owner November 28, 2024 18:06
@hughns hughns added the X-Needs-Product More input needed from the Product team label Nov 29, 2024
hughns
hughns requested changes Nov 29, 2024
View reviewed changes
Copy link
Member

@hughns hughns left a comment

Choose a reason for hiding this comment

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

Three things:

  • the test doesn't pass
  • I think there is a simpler way to do this just at the UrlParams level
  • the documentation update is missing.

As I am off today I will do a quick alternative PR.

hughns
hughns reviewed Nov 29, 2024
View reviewed changes
src/room/MuteStates.ts Outdated Show resolved Hide resolved
@hughns
Copy link
Member

hughns commented Nov 29, 2024

Actually, I've run out of time to do something today. What I noticed was that the URL parameters were not being parsed from the mocked router URL.

@hughns hughns changed the title allow join unmuted in widget mode Allow join unmuted in widget mode Nov 29, 2024
@toger5
Copy link
Contributor Author

toger5 commented Nov 29, 2024

Thanks for the finding about the router URL.
The tests work locally lets see what i have setup differently!

toger5
toger5 commented Dec 2, 2024
View reviewed changes
src/room/MuteStates.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hughns hughns hughns requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
X-Needs-Product More input needed from the Product team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@toger5 @hughns