Refactor common logic in core.ts

[BassemHalim]

What type of PR is this? (check all applicable)

• ✨ Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• ♻️ Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert
• 🌐 Internationalization / Translation

Description

• Refactor repeated code from handleAppMenuProject, handleAppMenuSettings, handleAppMenuHelp into setupAppMenu()
• Refactor repeated code from handleResultPopup, handleResultZoom and handleBroadcastStatus

This is a very small change (it only reduced line count by 1 :) ) but it's a first step towards refactoring core.ts

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help

Added to documentations?

• 📓 docs (./docs)
• 📕 storybook (./storybook)
• 📜 README.md
• 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

Summary by CodeRabbit

• Refactor
  • Centralized app menu initialization into a single setup path for all menus, unifying translation and toggle behavior.
  • Ensures consistent key-label translation across platforms and adds optional font-size adjustment for menu content.
  • Introduces a standardized tool-button creation helper and updates several UI buttons to use it, reducing duplication.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[netlify]

✅ Deploy Preview for livecodes ready!

Name	Link
🔨 Latest commit	d18fa4d
🔍 Latest deploy log	https://app.netlify.com/projects/livecodes/deploys/69362a3f3d7d4c0008201c7b
😎 Deploy Preview	https://deploy-preview-928--livecodes.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Walkthrough

Introduces setupAppMenu(container, button, menuHTML, shouldAdjustFontSize = false) to centralize app-menu HTML translation/injection, macOS key-label replacement, optional font-size adjustment, and button wiring; adds createToolButton helper and refactors several tool-button creations to use it in the same file.

Changes

Cohort / File(s)

Summary

Menu & tool-button refactor src/livecodes/core.ts

Adds setupAppMenu(container, button, menuHTML, shouldAdjustFontSize = false) to consolidate menu HTML translation/injection, macOS Ctrl-label replacement, optional font-size adjustment, and registerMenuButton wiring. Replaces per-menu inline handlers (handleAppMenuProject, handleAppMenuSettings, handleAppMenuHelp) with calls to the helper. Adds createToolButton and refactors tool-button creation usages (e.g., in result popup/zoom/broadcast status) to use the new helper.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Areas to inspect:
  • Correctness and expected behavior of replaceAll for macOS key-label replacement across target browsers.
  • That shouldAdjustFontSize is applied solely where intended (settings menu).
  • createToolButton integration points (tool-button markup, event wiring, and insertion locations).

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Refactor common logic in core.ts' accurately reflects the main change: refactoring duplicated menu handler logic into a centralized setupAppMenu() function, with additional standardization of tool-button creation via createToolButton().
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/livecodes/core.ts (1)

3133-3168: Excellent refactoring!

The extraction of common menu setup logic into setupAppMenu() successfully reduces code duplication and centralizes the menu initialization pattern. The function signature is clear, the default parameter shouldAdjustFontSize = false correctly maintains existing behavior, and the early null-check prevents errors.

Optional: Consider using replaceAll() for clarity.

On line 3158, you could modernize the keyboard shortcut replacement:

-  const html = isMac() ? menuHTML.replace(/<kbd>Ctrl<\/kbd>/g, '<kbd>⌘</kbd>') : menuHTML;
+  const html = isMac() ? menuHTML.replaceAll('<kbd>Ctrl</kbd>', '<kbd>⌘</kbd>') : menuHTML;

This is clearer and avoids regex escaping, though the current implementation works correctly.

Note: The static analysis warning about innerHTML (line 3160) is a false positive—the HTML content originates from trusted internal constants (menuProjectHTML, menuSettingsHTML, menuHelpHTML) imported from ./html, not user input.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8233cc4 and bae864b.

📒 Files selected for processing (1)

• src/livecodes/core.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/livecodes/core.ts (2)

src/livecodes/html/index.ts (3)

• menuProjectHTML (128-128)
• menuSettingsHTML (129-129)
• menuHelpHTML (127-127)

src/livecodes/utils/utils.ts (1)

• isMac (70-71)

🪛 ast-grep (0.40.0)

src/livecodes/core.ts

[warning] 3159-3159: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: container.innerHTML = html
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)

---

[warning] 3159-3159: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: container.innerHTML = html
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)

🪛 GitHub Check: SonarCloud Code Analysis

src/livecodes/core.ts

[warning] 3158-3158: Prefer String#replaceAll() over String#replace().

See more on https://sonarcloud.io/project/issues?id=live-codes_livecodes&issues=AZr6rUxTcqSqORs_W3yK&open=AZr6rUxTcqSqORs_W3yK&pullRequest=928

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

• GitHub Check: Redirect rules - livecodes
• GitHub Check: Header rules - livecodes
• GitHub Check: Pages changed - livecodes
• GitHub Check: Codacy Static Code Analysis
• GitHub Check: tests (24.x, 4)
• GitHub Check: tests (24.x, 5)
• GitHub Check: tests (24.x, 2)
• GitHub Check: tests (24.x, 3)
• GitHub Check: tests (24.x, 1)
• GitHub Check: build (24.x)
• GitHub Check: build

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud