-
Notifications
You must be signed in to change notification settings - Fork 825
fix: non-functional switch in multimeter screen #3009
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
base: flutter
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideConnects the multimeter UI switch to its provider state and renames the provider method for clarity. Sequence diagram for multimeter switch toggle flowsequenceDiagram
actor User
participant MultimeterScreen
participant SwitchWidget
participant MultimeterStateProvider
User->>SwitchWidget: tap
SwitchWidget->>MultimeterScreen: onChanged(value)
MultimeterScreen->>MultimeterStateProvider: setSwitch(value)
MultimeterStateProvider->>MultimeterStateProvider: update isSwitchChecked
MultimeterStateProvider-->>MultimeterScreen: notifyListeners()
MultimeterScreen->>SwitchWidget: rebuild with provider.isSwitchChecked
Updated class diagram for multimeter state and screenclassDiagram
class MultimeterStateProvider {
bool isSwitchChecked
void setSwitch(bool checked)
void notifyListeners()
}
class MultimeterScreen {
+build(BuildContext context)
}
class SwitchWidget {
+bool value
+Function onChanged
}
MultimeterScreen --> MultimeterStateProvider : uses
MultimeterScreen o-- SwitchWidget : contains
SwitchWidget --> MultimeterStateProvider : calls setSwitch
File-Level Changes
Assessment against linked issues
Possibly 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 - I've left some high level feedback:
- You can simplify the
onChangedcallback by passing the method directly (e.g.,onChanged: provider.setSwitch) instead of wrapping it in a one-line closure. - Consider renaming
setSwitchto something more explicit likesetSwitchCheckedto better convey that it updates a boolean checked state rather than a generic switch object.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You can simplify the `onChanged` callback by passing the method directly (e.g., `onChanged: provider.setSwitch`) instead of wrapping it in a one-line closure.
- Consider renaming `setSwitch` to something more explicit like `setSwitchChecked` to better convey that it updates a boolean checked state rather than a generic switch object.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Build StatusBuild successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/20694791565/artifacts/5018015443. Screenshots |







-1_instruments_screen.png?raw=true)
-2_nav_drawer.png?raw=true)
-3_accelerometer.png?raw=true)
-4_power_source.png?raw=true)
-5_multimeter.png?raw=true)
-6_wave_generator.png?raw=true)
-7_oscilloscope.png?raw=true)
Fixes #3008
Changes
Screenshots / Recordings
Screen_recording_20251222_223954.mp4
Checklist:
constants.dartor localization files instead of hard-coded values.dart formator the IDE formatter.flutter analyzeand tests run influtter test.Summary by Sourcery
Fix the multimeter screen switch so that toggling it updates the underlying state.
Bug Fixes:
Enhancements: