-
Notifications
You must be signed in to change notification settings - Fork 72
feat: Add native menu support for web modals in Electron app #542
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
Conversation
Enable Electron users to access web-based features (STH disc/tape archives, configuration, help) via native menus. Changes include: - Add menu items for STH disc/tape archives, configuration, and reset actions - Use contextBridge for secure IPC between main and renderer processes - Handle show-modal and action IPC messages to trigger Bootstrap modals - Fix STH archive loading in Electron by using https:// when protocol is file:// - Fix window.onresize bug by using dispatchEvent instead of direct call 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Display the BBC model name (e.g., "BBC Master 128") in the Electron window title bar as "jsbeeb - BBC Master 128". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Observe the .bbc-model DOM element for changes instead of having main.js explicitly call a setTitle function. This keeps the coupling one-directional. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@claude what do you reckon? |
|
Claude finished @mattgodbolt's task —— View job Reviewing PR: Add native menu support for web modals in Electron app
Review SummaryThis is an excellent implementation that successfully bridges Electron's native menus with the web-based modal system. The changes are well-structured, secure, and follow the repository's coding standards. ✅ StrengthsSecurity & Architecture:
Code Quality:
Feature Implementation:
🔍 Minor ObservationsPotential Areas for Enhancement:
Code Style Notes:
🚀 Technical HighlightsSmart Protocol Handling ( const protocol = document.location.protocol === "file:" ? "https:" : document.location.protocol;This elegantly solves the Electron Secure IPC Bridge ( contextBridge.exposeInMainWorld("electronAPI", {
onLoadDisc: (callback) => ipcRenderer.on("load", (event, message) => callback(message)),
// ... other secure API methods
});Perfect implementation of Electron's recommended security practices. Dynamic Title Updates ( const updateTitle = () => api.setTitle(`jsbeeb - ${modelElement.textContent}`);
new MutationObserver(updateTitle).observe(modelElement, {
childList: true, characterData: true, subtree: true
});Efficient solution for keeping window title in sync with emulated model. 🎯 Overall AssessmentThis PR successfully delivers native Electron menu integration while maintaining excellent code quality and security standards. The implementation is production-ready and adds significant value to the Electron desktop experience. Recommendation: ✅ Approve and Merge The code demonstrates thoughtful architecture, proper security practices, and clean implementation. All changes align well with the existing codebase patterns and conventions outlined in CLAUDE.md. |
Summary
file://protocol and usinghttps://explicitlywindow.onresize is not a functionbug🤖 Generated with Claude Code