refactor: vue ts and vue's reactive : src/simulator/src/themer/customThemer.vue #461
Conversation
WalkthroughThis pull request removes the customThemer.js file that managed theme customization using dynamic DOM generation and event listeners, and introduces a new Vue component, customThemer.vue, to handle custom color themes. The Vue component employs reactive state management and offers features for theme updates, applying themes with local storage persistence, and importing/exporting theme configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as CustomColorThemes (Vue Component)
participant LS as LocalStorage
participant GT as Global Theme Options
U->>V: Modify color input
V->>GT: Update specific theme property
U->>V: Click "Apply Theme"
V->>LS: Save theme configuration
V->>GT: Refresh global theme style
U->>V: Click "Import Theme"
V->>U: Show file selection dialog
U->>V: Select JSON file
V->>V: Read and validate JSON file
V->>GT: Update theme with imported values
U->>V: Click "Export Theme"
V->>U: Download JSON file of current theme
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/simulator/src/themer/customThemer.vue (5)
3-15: Prevent form submission and improve accessibility.The form element should prevent default submission, and the hidden elements should use CSS classes instead of inline styles.
Apply these changes:
- <form ref="propertiesContainer"> + <form ref="propertiesContainer" @submit.prevent> <div v-for="(property, key) in customTheme" :key="key" class="property"> - <label :for="String(key)">{{ key }} ({{ property.description }})</label> + <label :for="key">{{ key }} ({{ property.description }})</label> <input type="color" - :name="String(key)" + :name="key" v-model="customTheme[key]?.color" class="customColorInput" @input="updateTheme" /> </div> - <a id="downloadThemeFile" style="display: none"></a> + <a id="downloadThemeFile" class="hidden"></a> </form>
17-26: Enhance button accessibility and styling consistency.The buttons lack proper accessibility attributes, and the file input uses inline styles.
Apply these changes:
- <button @click="applyTheme">Apply Theme</button> - <button @click="importTheme">Import Theme</button> - <button @click="exportTheme">Export Theme</button> + <button @click="applyTheme" aria-label="Apply custom theme">Apply Theme</button> + <button @click="importTheme" aria-label="Import theme from file">Import Theme</button> + <button @click="exportTheme" aria-label="Export theme to file">Export Theme</button> <input type="file" id="importThemeFile" - style="display: none" + class="hidden" @change="handleFileImport" + accept=".json" />
41-44: Improve type safety and readability.The type casting and reactive state initialization could be simplified using proper TypeScript interfaces.
Consider adding these type definitions:
interface ThemeProperty { color: string; description: string; ref: string[]; } interface CustomTheme { [key: string]: ThemeProperty; }Then simplify the initialization:
- const customTheme = reactive<{ [key: string]: { color: string; description: string; ref: string[] } }>(CreateAbstraction(themeOptions["Custom Theme"]) as { [key: string]: { color: string; description: string; ref: string[] } }); + const customTheme = reactive<CustomTheme>(CreateAbstraction(themeOptions["Custom Theme"]) as CustomTheme);
47-47: Document magic values in updateBG function.The
dotsfunction is called with magic boolean values that need documentation.- const updateBG = () => dots(true, false, true); + // Update background with forcefully=true, gridUpdate=false, dots=true + const updateBG = () => dots(true, false, true);
127-135: Add missing .hidden class.Add the .hidden class used in the template for hidden elements.
<style scoped> .property { margin-bottom: 10px; } .customColorInput { margin-left: 10px; } + +.hidden { + display: none; +} </style>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/simulator/src/themer/customThemer.js(0 hunks)src/simulator/src/themer/customThemer.vue(1 hunks)
💤 Files with no reviewable changes (1)
- src/simulator/src/themer/customThemer.js
🔇 Additional comments (1)
src/simulator/src/themer/customThemer.vue (1)
31-35: Verify imported dependencies.Let's ensure all imported dependencies are available and up-to-date.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for the existence and exports of imported files echo "Checking for imported files..." fd -e ts -e js "canvasApi|themes|themer|customThemeAbstraction" src/simulator/src/themer/ # Check for exported symbols echo "Checking for exported symbols..." ast-grep --pattern 'export const dots' src/simulator/src/canvasApi.ts ast-grep --pattern 'export default themeOptions' src/simulator/src/themer/themes.ts ast-grep --pattern 'export function updateThemeForStyle' src/simulator/src/themer/themer.ts ast-grep --pattern 'export function CreateAbstraction' src/simulator/src/themer/customThemeAbstraction.tsLength of output: 960
🏁 Script executed:
#!/bin/bash # Check for the canvasApi file in the expected location relative to customThemer.vue (one directory up from themer) echo "Searching for canvasApi file in src/simulator/src/ ... " fd -e ts -e js "^canvasApi" src/simulator/src/ # Check for themes file in the current directory but as a .js file, not .ts. echo "Searching for themes file in src/simulator/src/themer/ ... " fd -e ts -e js "^themes" src/simulator/src/themer/ # Check for themer file in the current directory but as a .js file, not .ts. echo "Searching for themer file in src/simulator/src/themer/ ... " fd -e ts -e js "^themer" src/simulator/src/themer/Length of output: 634
🏁 Script executed:
#!/bin/bash echo "Checking exports in canvasApi.js..." ast-grep --pattern 'export const dots' src/simulator/src/canvasApi.js echo "Checking exports in themes.js..." ast-grep --pattern 'export default themeOptions' src/simulator/src/themer/themes.js echo "Checking exports in themer.js..." ast-grep --pattern 'export function updateThemeForStyle' src/simulator/src/themer/themer.js echo "Checking exports in customThemeAbstraction.ts..." ast-grep --pattern 'export function CreateAbstraction' src/simulator/src/themer/customThemeAbstraction.tsLength of output: 697
Imported dependencies verified successfully.
All required files exist at the expected locations, and their exports (dots, themeOptions, updateThemeForStyle, and CreateAbstraction) are properly defined. No issues were found with the import statements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/simulator/src/themer/customThemer.vue (5)
3-10: Add form submission prevention and improve accessibility.The form element should prevent default submission and needs accessibility improvements.
- <form ref="propertiesContainer"> + <form ref="propertiesContainer" @submit.prevent> <div v-for="(property, key) in customTheme" :key="key" class="property"> - <label :for="String(key)">{{ key }} ({{ property.description }})</label> + <label :for="'theme-' + key">{{ key }} ({{ property.description }})</label> <input type="color" :name="String(key)" v-model="customTheme[key]?.color" class="customColorInput" + :id="'theme-' + key" + :aria-label="`Choose color for ${key}`" @input="updateTheme" /> </div> <a id="downloadThemeFile" style="display: none"></a> </form>
12-14: Add descriptive aria-labels to buttons.Improve accessibility by adding descriptive aria-labels to the action buttons.
- <button @click="applyTheme">Apply Theme</button> - <button @click="importTheme">Import Theme</button> - <button @click="exportTheme">Export Theme</button> + <button @click="applyTheme" aria-label="Apply current theme">Apply Theme</button> + <button @click="importTheme" aria-label="Import theme from file">Import Theme</button> + <button @click="exportTheme" aria-label="Export current theme to file">Export Theme</button>
31-31: Improve type safety by extracting the type definition.The inline type assertion could be improved by extracting the type definition.
+ type CustomThemeType = { + [key: string]: { + color: string; + description: string; + ref: string[]; + }; + }; + - const customTheme = reactive<{ [key: string]: { color: string; description: string; ref: string[] } }>(CreateAbstraction(themeOptions["Custom Theme"]) as { [key: string]: { color: string; description: string; ref: string[] } }); + const customTheme = reactive<CustomThemeType>(CreateAbstraction(themeOptions["Custom Theme"]) as CustomThemeType);
84-94: Add error handling to export function.The export function should handle potential errors when generating the download URL.
const exportTheme = () => { - const dlAnchorElem = document.getElementById("downloadThemeFile") as HTMLAnchorElement; - dlAnchorElem.setAttribute( - "href", - `data:text/json;charset=utf-8,${encodeURIComponent( - JSON.stringify(themeOptions["Custom Theme"]) - )}` - ); - dlAnchorElem.setAttribute("download", "CV_CustomTheme.json"); - dlAnchorElem.click(); + try { + const dlAnchorElem = document.getElementById("downloadThemeFile") as HTMLAnchorElement; + if (!dlAnchorElem) { + throw new Error("Download element not found"); + } + const themeData = JSON.stringify(themeOptions["Custom Theme"]); + const dataUrl = `data:text/json;charset=utf-8,${encodeURIComponent(themeData)}`; + dlAnchorElem.setAttribute("href", dataUrl); + dlAnchorElem.setAttribute("download", "CV_CustomTheme.json"); + dlAnchorElem.click(); + } catch (error) { + console.error('Failed to export theme:', error); + alert("Failed to export theme!"); + } };
166-174: Add responsive design improvements.Consider adding responsive styles for better mobile experience.
<style scoped> .property { margin-bottom: 10px; + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 10px; } .customColorInput { - margin-left: 10px; + min-width: 44px; + min-height: 44px; } + +@media (max-width: 768px) { + .property { + flex-direction: column; + align-items: flex-start; + } +} </style>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/simulator/src/themer/customThemer.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - circuitverse
- GitHub Check: Header rules - circuitverse
- GitHub Check: Pages changed - circuitverse
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
src/simulator/src/themer/customThemer.vue (4)
20-26: LGTM!The imports are well-organized and TypeScript is properly configured.
36-58: LGTM!The theme update functions are well-implemented with proper error handling and type safety.
59-75: LGTM!The theme application function includes proper localStorage checks and error handling with fallback.
147-164: LGTM!The component lifecycle is properly managed using onMounted hook and all necessary methods are exported.
Fixes#414
Fixes#433
@JoshVarga
Summary by CodeRabbit