-
Notifications
You must be signed in to change notification settings - Fork 87
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
frontend:DefaultParamLoader: Fix to new parameter #3133
frontend:DefaultParamLoader: Fix to new parameter #3133
Conversation
* Fix parameter table to show new parameter instead of current one
Reviewer's Guide by SourceryThis pull request fixes the display issue in the parameter table by ensuring that the new parameter value is shown instead of the current one. The implementation modifies both the frontend Vue template and the logic that maps parameter values, creating a new parameter object with the desired value. Class diagram for DefaultParamLoader and parameter object structureclassDiagram
class DefaultParamLoader {
+mappedParameters(): ParameterItem[]
+printParamWithUnit(param): String
}
class ParameterItem {
+name: String
+current: Parameter
+value: Parameter
}
class Parameter {
+value: Any
...
}
DefaultParamLoader --> ParameterItem : creates
ParameterItem --> Parameter : current
ParameterItem --> Parameter : value
%% Note: The new change constructs a new Parameter object for value,
%% so that in the template the access is done via item.value.value
note for ParameterItem "'value' now holds a Parameter object instead of a primitive value"
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @JoaoMario109 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider renaming or restructuring the new parameter object to avoid the confusing double use of the 'value' key (e.g., item.value.value).
- Extract the duplicated template logic into a computed property to reduce repetition in handling current versus new parameter display.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fix parameter table to show new parameter instead of current one
Fix #3130
Summary by Sourcery
Bug Fixes: