-
Notifications
You must be signed in to change notification settings - Fork 139
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
Combining Vue Simulator Versioning PRs #337
Combining Vue Simulator Versioning PRs #337
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
// const { activeCircuit } = toRefs(simulatorStore) | ||
name = name || 'Untitled' | ||
name = stripTags(name) | ||
scopeList[id].name = name |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
user controlled input
scope.backups[scope.backups.length - 1] !== backup | ||
) { | ||
scope.backups.push(backup) | ||
scope.history = [] |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
user controlled input
) { | ||
scope.backups.push(backup) | ||
scope.history = [] | ||
scope.timeStamp = new Date().getTime() |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
user controlled input
*/ | ||
function loadModule(data, scope) { | ||
// Create circuit element | ||
var obj = new modules[rectifyObjectType(data.objectType)]( |
Check failure
Code scanning / CodeQL
Unvalidated dynamic method call High
user-controlled
… openProjectOffline
WalkthroughThe changes introduce a new Vue component, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OpenOffline
participant SimulatorState
User->>OpenOffline: Opens offline project dialog
OpenOffline->>SimulatorState: Fetches project list
SimulatorState-->>OpenOffline: Returns project list
User->>OpenOffline: Selects a project
OpenOffline->>OpenOffline: Checks project version
alt Version mismatch
OpenOffline-->>User: Displays version mismatch dialog
User->>OpenOffline: Chooses to continue or cancel
else No mismatch
OpenOffline-->>User: Loads selected project
end
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (2)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
v0/src/components/DialogBox/OpenOffline.vue (3)
33-35
: Prevent click event on delete icon from selecting the radio buttonClicking the delete icon inside the label may inadvertently select the radio button due to event propagation. To prevent this, use the
.stop
modifier on the@click
event.Apply this change:
Line 35~:
- @click="deleteOfflineProject(projectId)" + @click.stop="deleteOfflineProject(projectId)"This ensures that clicking the delete icon does not affect the radio button selection.
45-60
: Use a computed property to check ifprojectList
is emptyComparing
JSON.stringify(projectList)
to'{}'
is inefficient and may lead to unexpected results. Instead, use a computed property to check ifprojectList
is empty.In your script, add the computed property:
+ import { computed } from '@vue/runtime-core' + const isProjectListEmpty = computed(() => { + return Object.keys(projectList.value).length === 0 + })Update the template to use this computed property:
Lines 37~:
- <p v-if="JSON.stringify(projectList) == '{}'"> + <p v-if="isProjectListEmpty">Lines 45~ to 60~:
<v-btn - v-if="JSON.stringify(projectList) != '{}'" + v-if="!isProjectListEmpty" class="messageBtn" block @click="openProjectOffline()" > open project </v-btn> <v-btn - v-else + v-if="isProjectListEmpty" class="messageBtn" block @click.stop="OpenImportProjectDialog" > open CV file </v-btn>This approach is cleaner and enhances readability.
101-101
: Useconst
instead ofvar
for variable declarationUsing
const
for variable declarations that are not reassigned promotes better coding practices and prevents accidental reassignment.Line 101~:
- var data = localStorage.getItem('projectList') + const data = localStorage.getItem('projectList')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so large that it is hard to review. It looks good from what I can tell. I would suggest merging and doing a bunch of testing, even writing tests if we can.
If you can resolve the conflicts this PR will merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This is an enormous commit. We need to resolve any issues that come up after merging. There isn't much of a better option. |
Fixes #372
Describe the changes you have made in this PR -
Combining all the PRs related to Vue simulator versioning
Screenshots of the changes (If any) -
#332 - Enable the Main simulator to dynamically load the Vue simulator.
#327 - build: script modification for different builds.
#327 - chore: adding versioned directory(v0 and v1).
Key Achievements:
Versioning Directories:
v0
,v1
, etc.) to manage multiple versions of the Vue simulator, allowing for independent development and maintenance of each version.Modifying the Build Script:
vite.config.v0.ts
,vite.config.v1.ts
) to ensure proper building and output management for each simulator version.Bash Script for Multi-Version Builds:
Hot-Swapping Vue Simulator Versions:
simver
query parameter, allowing users to seamlessly switch between different versions.Dynamic Script Injection:
createHtmlPlugin
in Vite, ensuring that the correct version-specific script tags are dynamically inserted into theindex.html
during the build process.Storing Simulator Version in Circuit Data:
circuit_data
to store the simulator version, enabling circuits to be loaded with the appropriate simulator version based on their creation version.Redirecting to Correct Simulator Version:
simulatorVersion
incircuit_data
, ensuring compatibility and functionality.Creation of
index-cv.html
:index-cv.html
to serve as the entry point for the main repository, while the defaultindex.html
continues to serve thecv-frontend-vue
repository, maintaining stability during development.Setting
v0
as Default Simulator:v0
as the default simulator version, ensuring consistency during the development process.Fixing Circuit Preview Image Issue:
Version-Specific Links for Launching Circuits:
Future Work:
/simulatorvue
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
New Features
Bug Fixes
Chores