-
Notifications
You must be signed in to change notification settings - Fork 603
Added color field in environment model and ui #922
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
|
@shashank-lol You can refer to the the feedback here - #601 (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.
Pull request overview
This PR adds a color selection feature for environments, allowing users to choose from a predefined palette of 8 colors that are displayed in the environment dropdown and sidebar. The implementation downgrades the freezed dependency from v3.x to v2.5.7 to enable model modifications, adds a color field to the EnvironmentModel, and implements UI components for color display and selection.
Key Changes:
- Added
colorfield toEnvironmentModelwith customColorConverterfor JSON serialization - Downgraded
freezedandfreezed_annotationpackages to v2.5.7 across multiple packages - Implemented color picker dialog and visual indicators in the UI
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/apidash_core/lib/models/environment_model.dart | Added Color field with ColorConverter for JSON serialization |
| packages/apidash_core/lib/models/environment_model.freezed.dart | Generated freezed code for updated EnvironmentModel with color field |
| packages/apidash_core/lib/models/environment_model.g.dart | Generated JSON serialization code for color field |
| packages/apidash_core/lib/consts.dart | Added kGlobalColor constant for default environment color |
| packages/apidash_core/pubspec.yaml | Downgraded freezed packages and openapi_spec version |
| packages/apidash_core/pubspec_overrides.yaml | Updated dependency path overrides (with Windows-specific separators) |
| packages/apidash_design_system/lib/widgets/popup_menu.dart | Added colorResolver parameter to display color dots in menu items |
| packages/apidash_design_system/lib/tokens/colors.dart | Added kEnvColors palette with 8 predefined colors |
| lib/providers/environment_providers.dart | Updated addEnvironment and updateEnvironment methods to support color parameter |
| lib/screens/envvar/environments_pane.dart | Added color picker dialog and random color assignment for new environments |
| lib/widgets/card_sidebar_environment.dart | Added color dot indicator next to environment names |
| lib/widgets/popup_menu_env.dart | Integrated color display in environment dropdown menu |
| lib/screens/common_widgets/environment_dropdown.dart | Passed color to environment popup menu |
| lib/consts.dart | Added editColor option to ItemMenuOption enum |
| test/models/environment_models.dart | Updated test models to include color field |
| packages/*/pubspec.yaml | Downgraded freezed dependencies across multiple packages |
| packages/*/pubspec_overrides.yaml | Updated path overrides with Windows-specific separators |
| pubspec.lock | Reflected dependency version changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| enabled: false, | ||
| ), | ||
| ], | ||
| color: kGlobalColor |
Copilot
AI
Dec 4, 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.
[nitpick] Missing trailing comma after the color parameter. Adding a trailing comma improves code formatting and makes version control diffs cleaner when adding new parameters.
| color: kGlobalColor | |
| color: kGlobalColor, |
| enabled: false, | ||
| ), | ||
| ], | ||
| color: kEnvColors[1] |
Copilot
AI
Dec 4, 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.
[nitpick] Missing trailing comma after the color parameter. Adding a trailing comma improves code formatting and makes version control diffs cleaner when adding new parameters.
| color: kEnvColors[1] | |
| color: kEnvColors[1], |
| path: ..\\better_networking | ||
| curl_parser: | ||
| path: ../curl_parser | ||
| path: ..\\curl_parser | ||
| genai: | ||
| path: ../genai | ||
| path: ..\\genai | ||
| har: | ||
| path: ../har | ||
| path: ..\\har | ||
| insomnia_collection: | ||
| path: ../insomnia_collection | ||
| path: ..\\insomnia_collection | ||
| postman: | ||
| path: ../postman | ||
| path: ..\\postman | ||
| seed: | ||
| path: ../seed | ||
| path: ..\\seed |
Copilot
AI
Dec 4, 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 path separators have been changed from forward slashes (/) to backslashes (\\). This is Windows-specific and will break on Linux and macOS. Dart/Flutter pubspec files should use forward slashes as they work across all platforms. Consider reverting these paths to use forward slashes (e.g., ../better_networking).
| const kEnvironmentVariableEmptyModel = | ||
| EnvironmentVariableModel(key: "", value: ""); | ||
| EnvironmentVariableModel(key: "", value: ""); |
Copilot
AI
Dec 4, 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.
Changed from const to non-const unnecessarily. Since kEnvironmentVariableEmptyModel is a constant expression with literal values, it should remain const for compile-time optimization. The removal of const here appears to be unintended.
| const ADPopupMenu( | ||
| {super.key, | ||
| this.value, | ||
| required this.values, | ||
| this.onChanged, | ||
| this.tooltip, | ||
| this.width, | ||
| this.isOutlined = false, | ||
| this.borderColor, | ||
| this.colorResolver}); |
Copilot
AI
Dec 4, 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.
[nitpick] Inconsistent indentation and formatting. The constructor parameters should follow a consistent indentation pattern, with each parameter on its own line indented properly. The closing brace and semicolon should also be properly aligned.
| const ADPopupMenu( | |
| {super.key, | |
| this.value, | |
| required this.values, | |
| this.onChanged, | |
| this.tooltip, | |
| this.width, | |
| this.isOutlined = false, | |
| this.borderColor, | |
| this.colorResolver}); | |
| const ADPopupMenu({ | |
| super.key, | |
| this.value, | |
| required this.values, | |
| this.onChanged, | |
| this.tooltip, | |
| this.width, | |
| this.isOutlined = false, | |
| this.borderColor, | |
| this.colorResolver, | |
| }); |
| ? "Global" | ||
| : getEnvironmentTitle(e.name).clip(30) | ||
| : getEnvironmentTitle(e.name).clip(30), | ||
| )) ?? |
Copilot
AI
Dec 4, 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.
[nitpick] Missing trailing comma after the closing parenthesis on line 35. This would improve formatting consistency. The line should end with a comma after the closing parenthesis of getEnvironmentTitle(e.name).clip(30).
| )) ?? | |
| ,)) ?? |
| int get hashCode => | ||
| environmentId.hashCode ^ variable.hashCode ^ isUnknown.hashCode; | ||
| } | ||
|
|
Copilot
AI
Dec 4, 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.
[nitpick] The ColorConverter class lacks documentation. Consider adding a doc comment explaining its purpose, such as:
/// Converts between Flutter's [Color] type and an integer representation for JSON serialization.
/// Uses ARGB32 format for storage.
class ColorConverter implements JsonConverter<Color, int> {| /// Converts between Flutter's [Color] type and an integer representation for JSON serialization. | |
| /// Uses ARGB32 format for storage. |
| required String id, | ||
| @Default("") String name, | ||
| @Default([]) List<EnvironmentVariableModel> values, | ||
| @ColorConverter() @Default(kGlobalColor) Color color |
Copilot
AI
Dec 4, 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.
[nitpick] Missing trailing comma after the color parameter. Adding a trailing comma improves code formatting consistency with Dart style guidelines.
| @ColorConverter() @Default(kGlobalColor) Color color | |
| @ColorConverter() @Default(kGlobalColor) Color color, |
| void updateEnvironment( | ||
| String id, { | ||
| String? name, | ||
| String? name, Color? color, |
Copilot
AI
Dec 4, 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.
[nitpick] The parameter formatting is inconsistent. The name and color parameters should be on separate lines or follow a consistent pattern. Currently name, Color? color, appears on one line while other parameters have different formatting.
| String? name, Color? color, | |
| String? name, | |
| Color? color, |
| dependency_overrides: | ||
| seed: | ||
| path: ../seed | ||
| path: ..\\seed |
Copilot
AI
Dec 4, 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 path separator has been changed from forward slash (/) to backslash (\\). This is Windows-specific and will break on Linux and macOS. Dart/Flutter pubspec files should use forward slashes as they work across all platforms. Consider reverting to ../seed.
| path: ..\\seed | |
| path: ../seed |
|
Review comment has not been addressed and it has also been found that this PR contains various changes that are not required to resolve the issue. |
PR Description
This pull request implements the color selection feature for an environment from a predefined set of 8 colors. This color will be visible in the Environment pane at the top right .
Implementation Details
Screenshots
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?