-
Notifications
You must be signed in to change notification settings - Fork 598
core functionalities of dashbot #887
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
Conversation
- Created a new Dashbot package with core functionalities. - Implemented Dashbot window model and notifier for state management. - Added UI components including Dashbot default page and home page. - Integrated routing for Dashbot with initial routes defined. - Included assets for Dashbot icons and set up pubspec.yaml. - Added tests for Dashbot window notifier to ensure functionality. - Established a basic README and CHANGELOG for the package.
…chat viewmodel to use the existing request model
…navigation handling
… with error color
…e files - Updated comments and JSON keys in various prompt files to replace "explnation" with "explanation". - Adjusted the response structure in services and tests to ensure consistency with the updated key. - Ensured all related tests reflect the corrected key for accurate validation.
…nd ListView inside
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.
Pull Request Overview
Copilot reviewed 180 out of 185 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| expect(initialState.width, 350); | ||
| expect(initialState.height, 450); |
Copilot
AI
Sep 29, 2025
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.
The test expects width 350 and height 450, but the actual DashbotWindowModel test shows default values of width 400 and height 515. This inconsistency suggests either the test data is wrong or there are different default values being used.
| expect(initialState.width, 350); | |
| expect(initialState.height, 450); | |
| expect(initialState.width, 400); | |
| expect(initialState.height, 515); |
| /// Provides `firstOrNull` for lists. | ||
| extension FirstOrNull<T> on List<T> { | ||
| T? get firstOrNull => isEmpty ? null : first; | ||
| } |
Copilot
AI
Sep 29, 2025
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 extension duplicates functionality that's available in Dart's collection package. Consider using import 'package:collection/collection.dart' and remove this custom extension to reduce code duplication.
| Curl({ | ||
| required this.method, | ||
| required this.uri, |
Copilot
AI
Sep 29, 2025
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.
The constructor now requires method parameter but removed the previous assertion validations. This could allow invalid HTTP methods to be created. Consider adding validation back or documenting the expected method values.
animator
left a comment
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.
LGTM 🚀
PR Description
Adds all core features of DashBot
Related Issues
Checklist
mainbranch before making this PRflutter upgradeand verify)flutter test) and all tests are passingAdded/updated tests?
We encourage you to add relevant test cases.
OS on which you have developed and tested the feature?