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

Add presenter overlay when sharing screen #10669

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Oct 9, 2023

☑️ Resolves

  • Fix Screensharing view does not show speaker #4478
  • You can hide and unhide by clicking on the presenter overlay
  • When hidden, a button will be shown.
  • If the video is disabled from the stripe, the presenter overlay will be hidden. Pressing on the presenter button will enable the video and show the overlay.

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Default 🏡 On hover 🏡 Clicked 🏡 Stripe Open
Presenter1 image Screenshot 2023-10-09 230256 Screenshot 2023-10-09 230406

🚧 Tasks

  • Design/icons review
  • Code review

🏁 Checklist

src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the feat/4478/show-speaker-when-screensharing branch from 0689a6c to 64b51a5 Compare October 13, 2023 07:26
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Looks cool code-wise and running! Nothing blocking from me.
Left some minor comments for improvements, and waiting for design team for decision

src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Show resolved Hide resolved
src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the feat/4478/show-speaker-when-screensharing branch 2 times, most recently from 8e6c3e0 to 7dd9732 Compare October 13, 2023 14:33
@DorraJaouad DorraJaouad force-pushed the feat/4478/show-speaker-when-screensharing branch from 7dd9732 to 7093fee Compare October 16, 2023 07:28
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Left some nitpick comments

src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
src/components/CallView/CallView.vue Outdated Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the feat/4478/show-speaker-when-screensharing branch from 7093fee to 875ee0e Compare October 16, 2023 17:05
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Very cool! :) @DorraJaouad only a very small adjustment: To make the hide icon more symmetric with the one to show the video, instead of the "eye" I'd propose account-off: https://pictogrammers.com/library/mdi/icon/account-off/

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the feat/4478/show-speaker-when-screensharing branch from 875ee0e to 2692b84 Compare October 18, 2023 19:38
@nickvergessen
Copy link
Member

So, good to merge?

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks great! :)

@DorraJaouad DorraJaouad merged commit c36a572 into master Oct 19, 2023
@DorraJaouad DorraJaouad deleted the feat/4478/show-speaker-when-screensharing branch October 19, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Screensharing view does not show speaker
4 participants