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.

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

[VEUE-864] Share Second Camera #563

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[VEUE-864] Share Second Camera #563

wants to merge 10 commits into from

Conversation

Sirbuland
Copy link
Collaborator

@Sirbuland Sirbuland commented Jun 10, 2021

PR Functionality

This PR adds the ability to share another camera at the current place of screen share in BBB. It provides with an option to select available cameras and use any one at the place of current primary area.

@HamptonMakes
Copy link
Contributor

Task linked: VEUE-864 Share Second Camera

@render
Copy link

render bot commented Jun 10, 2021

Your Render PR Server URL is https://stage-pr-563.onrender.com.

Follow its progress at https://dashboard.render.com/web/srv-c30uqfs64ckh7frfg3tg.

private boundUseStartOffset: EventListener;

element!: HTMLElement;
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the declare for?

Copy link
Collaborator Author

@Sirbuland Sirbuland Jun 22, 2021

Choose a reason for hiding this comment

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

@hcatlin so I faced this error
TypeError: Cannot set property which has only a getter for our targets in controllers
This was the issue if you set useDefineForClassFields to true in tsconfig file. But this flag was causing a lot of external libs to fail too! So I removed this flag and MobX worked without it (it was recommended on their official installation guide). Install Guide.

But for typescript it will be making it mandatory to not use class level declarations if an accessor/getter is already defined in base class. Also this is becoming default for ESNext. Further details are in this issue: microsoft/TypeScript#27644

Btw here my declare is not doing much but I leave it (even after removing flag) there just because of my above understanding and search. Please let me know if we should keep it or remove it for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hcatlin also I read this blog for solution https://dieterlunn.ca/stimulus-and-typescript/

Copy link
Contributor

Choose a reason for hiding this comment

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

You can keep it, I was more just curious!

Copy link
Collaborator

@KonnorRogers KonnorRogers left a comment

Choose a reason for hiding this comment

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

1 comment on profile_edit, everything else looks good to me.

Make sure to rebase this off main so tests will pass, also make sure to run prettier! Please and thank you :)

app/javascript/style/components/_video_card.scss Outdated Show resolved Hide resolved
app/javascript/controllers/profile_edit_controller.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants