Skip to content

Add Visual Theme Builder Pro Feature - #355

Draft
estruyf with Copilot wants to merge 12 commits into
devfrom
copilot/add-pro-theme-builder-feature
Draft

Add Visual Theme Builder Pro Feature#355
estruyf with Copilot wants to merge 12 commits into
devfrom
copilot/add-pro-theme-builder-feature

Fix unused properties check and improve documentation clarity

f089ecc
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - Correctness Check succeeded Jan 2, 2026 in 2m 12s

No issues identified (27 code objects reviewed).

• Merge Base: 5921c06
• Head: f089ecc

Details

✅ File Path Comments Posted
✅ apps/vscode-extension/src/extension.ts 0
✅ apps/vscode-extension/src/models/WebviewType.ts 0
✅ apps/vscode-extension/src/panels/ThemeBuilderPanel.ts 0
✅ apps/vscode-extension/src/services/ThemeBuilderService.ts 0
✅ apps/webviews/src/components/webviews/ThemeBuilderView.tsx 0
✅ apps/webviews/src/main.tsx 0
✅ packages/common/src/constants/Command.ts 0
✅ packages/common/src/constants/WebViewMessages.ts 0

Filtered Issues Details

apps/vscode-extension/src/panels/ThemeBuilderPanel.ts
  • line 32: The overridden messageListener does not call super.messageListener(message) for unhandled commands. The base class BaseWebview.messageListener handles essential commands like getSetting, getFileContents, and runCommand. By not calling the parent implementation, these commands will silently fail when sent to ThemeBuilderPanel. [ Already posted ]
  • line 36: In messageListener, payload is destructured from message but is passed to handlers like loadTheme, loadLayout, saveTheme, saveLayout, and getPreviewHtml without validating that payload is defined. If a message arrives without a payload property, these handlers will receive undefined and crash when accessing payload.name or payload.content (e.g., line 144 in loadTheme accesses payload.name). [ Already posted ]
  • line 84: The getExistingThemes method returns a path property in each theme object that contains just the filename, but loadTheme reconstructs the path using only payload.name and ignores this path field. If a theme filename differs from the sanitized name (e.g., due to special characters), the load will fail because the wrong path is constructed. [ Low confidence ]
  • line 88: In getExistingThemes, file.replace('.css', '') on line 88 will replace all occurrences of .css in the filename, not just the extension. A file named my.css.theme.css would produce a name of mytheme instead of my.css.theme. Use file.replace(/\.css$/, '') for safer extension removal. [ Already posted ]
  • line 153: In loadTheme, loadLayout, saveTheme, and saveLayout, the code accesses payload.name and payload.content without validating that payload is defined. If the webview sends a malformed message with payload as null or undefined, accessing payload.name will throw a TypeError: Cannot read properties of undefined. [ Already posted ]
  • line 162: In loadTheme, if payload is null or undefined (e.g., due to a malformed message from the webview), accessing payload.name on line 162 will throw a TypeError: Cannot read properties of null/undefined (reading 'name'). The function lacks validation that payload exists before accessing its properties. [ Already posted ]
  • line 187: The loadLayout method hardcodes the .hbs extension when constructing the file path (line 187: `${payload.name}.hbs`), but getExistingLayouts returns layouts with either .hbs or .handlebars extensions. When a user selects a layout file that was originally named with .handlebars extension, the load will fail because the code looks for .hbs instead of using the original extension. [ Already posted ]
  • line 199: In loadLayout, the path is constructed with a hardcoded .hbs extension (${payload.name}.hbs on line 199), but getExistingLayouts lists files with both .hbs and .handlebars extensions (line 125). When a user selects a .handlebars file from the list and attempts to load it, the function will look for a non-existent .hbs file and return null, silently failing to load the layout. [ Already posted ]
  • line 199: In loadLayout, if payload is null or undefined, accessing payload.name on line 199 will throw a TypeError. The function lacks validation that payload exists before accessing its properties. [ Already posted ]
  • line 242: In saveTheme, if payload is null or undefined, accessing payload.name on line 242 will throw a TypeError. The function lacks validation that payload and its required properties (name, content) exist. [ Already posted ]
  • line 281: In saveLayout, if payload is null or undefined, accessing payload.name on line 281 will throw a TypeError. The function lacks validation that payload and its required properties (name, content) exist. [ Already posted ]
apps/webviews/src/components/webviews/ThemeBuilderView.tsx
  • line 80: The state variable previewHtml and its setter setPreviewHtml are declared but never used anywhere in the component. While not a runtime crash, this is dead code that should be removed. [ Low confidence ]
  • line 401: When the user selects the placeholder option "Select existing theme..." (which has value=""), the onChange handler calls loadTheme("") with an empty string. This triggers an async request to load a theme with an empty name, which will likely fail or return unexpected data from the backend. [ Already posted ]
  • line 583: After calling removeTemplate(activeTemplate), the code reads Object.keys(themeConfig.slideTemplates)[0] to set the new active template. However, themeConfig is captured from the closure and still contains the old state (before the template was removed). This can result in activeTemplate being set to the template that was just removed, causing the UI to attempt to render a non-existent template configuration. [ Already posted ]
View more details on MacroscopeApp