Skip to content

fix(sessions): harden DataSaverToggle with cn styling, testids & comprehensive unit tests - #7136

Merged
github-actions[bot] merged 2 commits into
SatyamPandey-07:mainfrom
saisarans2007:fix/harden-datasaver-toggle-7135
Nov 1, 2026
Merged

github-actions[bot] merged 2 commits into
SatyamPandey-07:mainfrom
saisarans2007:fix/harden-datasaver-toggle-7135

Conversation

@saisarans2007

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Component Hardening & Styling with cn:
    • Replaced manual template string concatenation with cn(...) from @/lib/utils for consistent class merging on both container and switch button.
    • Exported DataSaverToggleProps interface with optional "data-testid" and className.
    • Added named and default exports for DataSaverToggle.
  • Accessibility & Testability Hooks:
    • Added data-testid hooks on root container (data-saver-toggle), switch button (data-saver-toggle-button), and status label (data-saver-status).
  • Comprehensive Unit Testing:
    • Added unit test suite in src/__tests__/components/sessions/DataSaverToggle.test.tsx (6 tests) verifying:
      • Named and default export equality.
      • Default off state rendering.
      • Uncontrolled toggle clicks and onChange callback execution.
      • Controlled enabled prop handling and synchronization.
      • Custom estimatedSavedMB prop rendering.
      • Custom className and data-testid propagation via cn().

Closes #7135

@vercel

vercel Bot commented Nov 1, 2026

Copy link
Copy Markdown

@saisarans2007 is attempting to deploy a commit to the pandeysatyam1802-gmailcom's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

🎉 Thank you for contributing to WorkSphere!
Please ensure:

  • ✅ Tests pass
  • ✅ No secrets committed
  • ✅ Documentation updated

💡 Connect & Support:

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved by CI: PR is linked to issue(s) #7135.

@github-actions github-actions Bot added good-pr good-ui OSCI'26 Open Source Contribution Initiative '26 needs-review frontend and removed needs-review labels Nov 1, 2026
@github-actions
github-actions Bot merged commit c11537c into SatyamPandey-07:main Nov 1, 2026
7 of 8 checks passed
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

🚀 Auto-merge complete!

This pull request is linked to issue(s) #7135 and has been automatically squash-merged into main with co-author credit! 🎉

Thank you for contributing to WorkSphere! ✨

github-actions Bot pushed a commit that referenced this pull request Nov 1, 2026
… reveal overlay controls on touch devices (closes #7133) (#7137)

…reveal overlay controls on touch devices

- Hide the viewer stage via a class instead of unmounting it, so the video keeps its srcObject after Show Stage (previously blank) and an active PiP window is not torn down
- Reveal the quality/crop overlay on hover-less (touch) devices, not only on hover/focus
- Drop the unused requestPip destructure flagged by eslint
- Add regression tests for video/stream persistence across Hide/Show Stage, PiP availability while hidden, and the touch overlay class

Closes #7133

  ## Description

  Completes #7133. Part of this issue landed in #7136 (linked to #7135), but that implementation unmounts the stage on **Hide Stage**. Because the `srcObject` effect only re-runs when `activeStream` changes, the remounted
  `<video>` comes back **blank** after **Show Stage**, and any active PiP window is torn down. The overlay controls are also still invisible on touch devices.

  Changes:
  1. **Hide Stage keeps the stage mounted** — hidden via `hidden` class (viewer only), so the same `<video>` keeps its stream and PiP keeps working.
  2. **Touch support** — overlay controls get `[@media(hover:none)]:opacity-100`, completing the "not hover-only" acceptance criterion.
  3. Removed the unused `requestPip` destructure (eslint warning under the zero-warning lint-staged hook).
  4. **Tests** — updated the Hide/Show Stage test and added regression tests: same video element + `srcObject` preserved across Hide → Show, PiP button available while hidden, touch overlay class. The new tests fail on current
  `main` (4 failures) and pass with this change.

  ## Related Issue

  Closes #7133

  ## Checklist

  - [x] My code follows the style guidelines of this project
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have made corresponding changes to the documentation
  - [x] My changes generate no new warnings
  - [x] I have added tests that prove my fix is effective or that my feature works
  - [x] New and existing unit tests pass locally with my changes
  - [ ] Any dependent changes have been merged and published in downstream modules

  ### Verification
  - lint-staged `eslint --max-warnings=0` ✅
  - `npx tsc --noEmit` — no errors in changed files (pre-existing errors elsewhere on `main`)
  - `npm test` — session suites 38/38 ✅
  - `npm run build` ✅

  ## Breaking Changes

  - [ ] Yes
  - [x] No

---

Co-authored-by: SatyamPandey-07 <186389297+SatyamPandey-07@users.noreply.github.com>

Co-authored-by: Satyam Pandey <satyam53@tsecol.onmicrosoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend good-pr good-ui OSCI'26 Open Source Contribution Initiative '26

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(sessions): harden DataSaverToggle with cn styling, testids & comprehensive unit tests

1 participant