Skip to content

feat: add javascript toggle controls for device previews#1481

Open
Dailin521 wants to merge 5 commits intoresponsively-org:mainfrom
Dailin521:codex/responsively-490-disable-javascript-remount-webview
Open

feat: add javascript toggle controls for device previews#1481
Dailin521 wants to merge 5 commits intoresponsively-org:mainfrom
Dailin521:codex/responsively-490-disable-javascript-remount-webview

Conversation

@Dailin521
Copy link
Copy Markdown
Contributor

Summary

Add JavaScript toggle controls for device previews without remounting the React Device tree.

Root cause

For #490, the webview must be recreated for JavaScript mode changes because webpreferences only take effect when the webview is created. The earlier implementation rebuilt the whole preview device subtree, which left stale listeners and stale webview references in follow-up flows.

Changes

  • keep each Device mounted and remount only the target webview when JavaScript is toggled
  • refresh toolbar webview references and scope listener cleanup to the current webview instance
  • keep screenshot actions disabled while JavaScript is disabled for a preview
  • add a regression test to ensure toggling JavaScript does not remount Device

Verification

  • npm run typecheck
  • npx eslint src/renderer/components/Previewer/Device/index.tsx src/renderer/components/Previewer/index.tsx src/renderer/components/Previewer/index.test.tsx
  • npm run test -- src/renderer/store/features/javascript/index.test.ts src/renderer/components/ToolBar/Menu/Flyout/JavaScriptControls/index.test.tsx src/renderer/components/Previewer/Device/Toolbar.test.tsx src/renderer/components/Previewer/index.test.tsx
  • npm run test
  • manual Electron verification on 2026-03-25:
    • per-device JavaScript toggle remounts only the target webview
    • all-previews JavaScript toggle remounts all webviews
    • Device roots remain mounted in both flows
    • screenshot actions are disabled while JavaScript is off

Closes #490

@Dailin521
Copy link
Copy Markdown
Contributor Author

Dailin521 commented Mar 25, 2026

Added the local runtime verification screenshots here for reviewer convenience.

These images are hosted in the dedicated public artifacts repository:

Per-device JavaScript toggle:
Real Electron verification passed. Disabled JavaScript on a single preview and confirmed only the targeted preview remounted.

Per-device JavaScript disabled verification

All previews JavaScript toggle:
Real Electron verification passed. Disabled JavaScript for all previews and confirmed the global control reached the expected disabled state.

All previews JavaScript disabled verification

@Dailin521
Copy link
Copy Markdown
Contributor Author

Quick follow-up here. CI is green and the current scope is still intentionally narrow, limited to the existing device preview JavaScript toggle paths. If this direction looks acceptable, I’m happy to make any small adjustments needed for merge.

@manojVivek
Copy link
Copy Markdown
Collaborator

@Dailin521 Sorry for the delay in reviewing this, can you please check and make sure we have e2e test cases to cover this feature here: https://github.com/responsively-org/responsively-app/tree/main/desktop-app/e2e/tests?

@Dailin521 Dailin521 force-pushed the codex/responsively-490-disable-javascript-remount-webview branch from f6691bd to cce53fb Compare April 17, 2026 03:55
@Dailin521
Copy link
Copy Markdown
Contributor Author

Added the requested E2E coverage on the updated head.

This now covers both paths under desktop-app/e2e/tests:

  • the global JavaScript (All Previews) toggle in the menu flyout
  • the per-preview Disable/Enable JavaScript action from the device toolbar overflow menu

The new tests also assert the related screenshot-disabled behavior and the preview remount behavior after toggling JavaScript.

Fresh Analyze and test (macos-latest) runs are already queued on this push.

@Dailin521
Copy link
Copy Markdown
Contributor Author

Pushed a follow-up fix for the first CI round. This only tightens the new E2E selectors so the test scopes to the first device toolbar more reliably and keeps the screenshot-all assertion stable when the disabled tooltip text changes. Fresh checks are running now.

@Dailin521
Copy link
Copy Markdown
Contributor Author

Pushed one more follow-up for the second CI round. The remaining failures were not selector-related anymore: on javascript=no webviews, Electron can reject executeJavaScript calls instead of returning a page-level undefined, so the new E2E assertions now treat that disabled-preview path as an expected execution failure signal and keep the enabled-preview checks intact. Fresh checks are running again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Turn off Javascript

2 participants