Dismiss video effects error when camera is off#3137
Conversation
prabhjot-msft
requested review from
AmyL219,
JamesBurnside,
JoshuaLai,
Leah-Xia-Microsoft,
PorterNan,
alkwa-msft,
carocao-msft,
dmceachernmsft,
edwardlee-msft,
emlynmac,
jimchou-dev,
jpeng-ms,
mgamis-msft,
palatter,
vhuseinova-msft and
waynemo
Chat bundle size is not changed.
|
CallWithChat bundle size is increased❗.
|
Calling bundle size is increased❗.
|
|
|
||
| /* @conditional-compile-remove(video-background-effects) */ | ||
| if (useIsParticularSidePaneOpen('videoeffects') && props.errorBarProps) { | ||
| if ((useIsParticularSidePaneOpen('videoeffects') || !isCameraOn) && props.errorBarProps) { |
There was a problem hiding this comment.
We haven't used this anywhere in the codebase yet, but instead of hiding the error when the camera is off, could we clear the error in the the handler for toggleCamera using newClearCallErrorsModifier?
Honestly no idea if that works just curious if its possible.
There was a problem hiding this comment.
The way its currently implemented, does the error re-show when the camera is turned back on?
There was a problem hiding this comment.
-
For 1st suggestion, using
newClearCallErrorsModifier, this modifier is not actually exposed in the calling stateful layer. If we are planning to expose this modifier, it would be called in the stateful layer mostly in the call context by introducing new set errors method. We can trigger this method whenever we want to delete certain errors. I can pursue this as a separate task if we feel strongly about this approach. -
For 2nd question, as part of the second change for auto-dismiss(Auto-dismiss video effect errors #3160). Whenever camera is turned on, it will reset a new active effect and if the error is generated again it would be shown and otherwise no error will be shown.
|
|
||
| /* @conditional-compile-remove(video-background-effects) */ | ||
| if (useIsParticularSidePaneOpen('videoeffects') && props.errorBarProps) { | ||
| if ((useIsParticularSidePaneOpen('videoeffects') || !isCameraOn) && props.errorBarProps) { |
There was a problem hiding this comment.
is this supposed to be !isCameraOn?
The title indicates that it should dismiss when camera is off, but this is setting an activemessage when camera is off.
I see that in videoEffectsPane.tsx line 149 new change it does what I expected.
Correct me if I'm wrong here.
There was a problem hiding this comment.
Yeah, so in this scenario, if videoeffectsSidepane is open and camera is off we are filtering out the video effects error from latest errors.
For videoeffectsPane, if the videoeffectsError exists and camera is on we are showing the error.
|
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
3 participants
What
Dismiss video effects error when camera is off
Why
https://skype.visualstudio.com/SPOOL/_workitems/edit/3288350
How Tested
Process & policy checklist
Is this a breaking change?