-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(keep-hand-raised-notification-when-the-hands-will-be-auto-lowered) #14960
base: master
Are you sure you want to change the base?
feat(keep-hand-raised-notification-when-the-hands-will-be-auto-lowered) #14960
Conversation
Hi, thanks for your contribution! |
clearTimeout(timeoutId); | ||
store.dispatch(hideNotification(LOWER_RAISED_HANDS_ID)); | ||
} ] | ||
}, NOTIFICATION_TIMEOUT_TYPE.MEDIUM)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this one short.
store.dispatch(raiseHand(false)); | ||
}, NOTIFICATION_TIMEOUT.MEDIUM); | ||
|
||
store.dispatch(showNotification({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a config flag for this, please.
7d70468
to
5a75989
Compare
config.js
Outdated
@@ -235,6 +235,10 @@ var config = { | |||
// Specifies whether the raised hand will hide when someone becomes a dominant speaker or not | |||
// disableRemoveRaisedHandOnFocus: false, | |||
|
|||
// Specifies whether there is a notification before hiding the raised hand | |||
// when someone becomes the dominant speaker. | |||
// disableRemoveRaisedHandOnFocusNotification: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's put the option inside the new raisedHands config object. Rebase this PR after the other lands and put it in there.
Let me know if you need help with that!
5a75989
to
d2cdea0
Compare
@@ -489,9 +489,9 @@ export interface IConfig { | |||
prejoinPageEnabled?: boolean; | |||
raisedHands?: { | |||
disableLowerHandByModerator?: boolean; | |||
disableLowerHandNotification?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This serves the same purpose as 'disableRemoveRaisedHandOnFocusNotification'. Since I think the new name is clearer, I’ve updated it accordingly. Please let me know any advice.
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Ops, this fell through the cracks. Can you give it one last look @mihhu ? |
Hi reviewer,
This is a GSoC task. I added a lower-hand notification before a certain participant's hands would be auto-lowered (when the dominant speaker changed). Feel free to let me know which part is inappropriate.
Thanks,
Mengyuan