feat: show loader during background effect initialization [WPB-24453]#21441
feat: show loader during background effect initialization [WPB-24453]#21441zskhan wants to merge 1 commit into
Conversation
zskhan
requested review from
arjita-mitra,
e-maad,
otto-the-bot,
screendriver and
thisisamir98
as code owners
|
|
🔗 Download Full Report Artifact 🧪 Playwright Test Summary
|
|
|
||
| const {name} = useKoSubscribableChildren(participant?.user, ['name']); | ||
|
|
||
| const isBackgroundEffectInitializing = useBackgroundEffectsStore(state => state.isInitializing); |
There was a problem hiding this comment.
could you add some tests for this state tansition? ideally unit, integration and e2e. Lets start with some unit tests?
- should show loading overlay when background effect is initializing
- should hide loading overlay when background effect is done initializing
- should show loading overlay while video is loading (hasActiveVideo && !isVideoReady)
- should hide loading overlay when video is ready (onCanPlay)
- should not show overlay for non-self participants
| const isSelfInitializing = | ||
| isSelfParticipant && isBackgroundEffectInitializing && is.nullOrUndefined(processedVideoStream); | ||
| const hasActiveVideo = (sharesCamera || sharesScreen) && !!videoStream && !isSelfInitializing; | ||
| const showLoadingOverlay = |
There was a problem hiding this comment.
This logic to showLoadingOverlay is a bit complex. we could extract it in a custom hook and that would make it easily testable too
// Simple, readable call to the custom hook
const showLoadingOverlay = useShowLoadingOverlay(
isSelfParticipant,
hasActiveVideo,
processedVideoStream,
);
| </div> | ||
| )} | ||
|
|
||
| {showLoadingOverlay && ( |
There was a problem hiding this comment.
Please test(cmd+F5) that the loading overlay is properly announced to screen readers. Consider adding aria-label or aria-busy attributes
2 participants
During the initialization and application of background effect, there will be a spinner displayed to improve the UX