Updated label value field story, selection card story and switch story #139
Conversation
WalkthroughThe changes in this pull request involve significant updates to several widget classes in the Flutter UI components. The Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (7)
flutter/digit-ui-components/storybook/lib/widgets/molecules/list_view_summary_stories.dart (2)
1-8: Remove unused importThe import for
label_value_list.darton line 2 appears to be unused in this file. Consider removing it to keep the imports clean and relevant.Apply this diff to remove the unused import:
-import 'package:digit_ui_components/widgets/atoms/label_value_list.dart';
110-113: Add placeholder onPressed callbacks for action buttonsThe action buttons have empty onPressed callbacks. Consider adding placeholder callbacks or logging statements to improve the interactive experience in the storybook.
Here's a suggested improvement:
action: action ? [ DigitButton( label: 'Cancel', onPressed: () => print('Cancel button pressed'), type: DigitButtonType.secondary, size: DigitButtonSize.large ), DigitButton( label: 'Action', onPressed: () => print('Action button pressed'), type: DigitButtonType.primary, size: DigitButtonSize.large ), ] : null,flutter/digit-ui-components/digit_components/lib/widgets/atoms/switch.dart (1)
140-140: Approve change with suggestions for improvementThe modification enhances the visual feedback for readonly and disabled states, which is a good improvement for accessibility and user experience.
However, there are a couple of suggestions to further improve the code:
Consider extracting the condition
widget.readonly || widget.disabledto a local variable to reduce duplication and improve readability. This condition is used multiple times in the widget.Add null checks for theme properties to improve robustness. The current implementation assumes that
theme.colorTheme.generic.divideris always non-null, which might not be true for all theme configurations.Here's a suggested refactor:
final isInactive = widget.readonly || widget.disabled; final dividerColor = theme.colorTheme.generic.divider; final symbolColor = DigitSwitchTheme?.symbolColor ?? defaultSwitchTheme.symbolColor; color: isInactive ? (dividerColor ?? Colors.grey) // Fallback color if dividerColor is null : (symbolColor ?? Colors.white), // Fallback color if symbolColor is nullThis refactoring improves code readability, reduces duplication, and adds null safety.
flutter/digit-ui-components/storybook/lib/widgets/molecules/view_card_stories.dart (1)
529-535: Good update toLabelValueSummary, consider diversifying example data.The change from
LabelValueListtoLabelValueSummaryand the use ofLabelValueIteminstead ofLabelValuePairappear to be positive improvements, likely offering more flexibility or enhanced functionality.However, to make the example more representative and clearer:
Consider diversifying the labels in the example items to better demonstrate real-world usage. For instance:
LabelValueSummary( items: [ LabelValueItem(label: 'Start Date', value: '22/03/2025'), LabelValueItem(label: 'End Date', value: '23/03/2025'), LabelValueItem(label: 'Duration', value: '1 day'), LabelValueItem(label: 'Location', value: 'Conference Room A'), LabelValueItem(label: 'Organizer', value: 'John Doe'), ], ),This change would better illustrate the component's capability to display varied information.
flutter/digit-ui-components/digit_components/lib/widgets/atoms/label_value_list.dart (1)
5-26: Add documentation comments for the class and its constructorAdding
///documentation comments to theLabelValueItemclass and its constructor parameters will improve code maintainability and help other developers understand the purpose and usage of this component.Apply this diff to add documentation:
+/// A widget that displays a label and its corresponding value with flexible layout options. class LabelValueItem extends StatelessWidget { final String label; final dynamic value; final TextStyle? labelTextStyle; final TextStyle? valueTextStyle; final bool isInline; final int labelFlex; final int valueFlex; final int? maxLines; final EdgeInsets? padding; + /// Creates a customizable label-value pair widget. const LabelValueItem({ Key? key, + /// The label to display. required this.label, + /// The value to display, which can be a [String], [List<String>], [Widget], or [List<Widget>]. required this.value, this.labelTextStyle, this.valueTextStyle, this.isInline = true, // Default to inline layout this.labelFlex = 2, this.valueFlex = 8, this.maxLines, this.padding, }) : super(key: key);flutter/digit-ui-components/digit_components/lib/widgets/atoms/selection_card.dart (2)
Line range hint
195-310: Refactor to eliminate code duplication in the build methodThe
buildmethod contains duplicated code in both branches of the conditional operator based onwidget.showParentContainer. This can make maintenance more challenging and increase the risk of inconsistencies.Extract the common code into a separate method or widget to enhance readability and maintainability. Here's how you might refactor the code:
Widget _buildErrorMessage(TextTheme textTheme, ThemeData theme) { return Column( children: [ const SizedBox(height: spacer1), Row( mainAxisSize: MainAxisSize.min, crossAxisAlignment: CrossAxisAlignment.start, children: [ Icon( Icons.info, color: theme.colorTheme.alert.error, size: BaseConstants.errorIconSize, ), const SizedBox(width: spacer1), Flexible( child: Text( widget.errorMessage!, style: textTheme.bodyLarge?.copyWith( color: theme.colorTheme.alert.error, ), ), ), ], ), ], ); } @override Widget build(BuildContext context) { final theme = Theme.of(context); final textTheme = theme.textTheme; bool isMobile = AppView.isMobileView(MediaQuery.of(context).size); if (widget.equalWidthOptions) { _calculateMaxOptionWidth(context); } Widget content = Wrap( alignment: WrapAlignment.center, spacing: spacer6, runSpacing: spacer6, children: widget.options.map(_buildOption).toList(), ); List<Widget> children = [content]; if (widget.errorMessage != null) { children.add(_buildErrorMessage(textTheme, theme)); } if (widget.showParentContainer) { return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ LabeledField( label: widget.title, isRequired: widget.isRequired, child: Container( width: isMobile ? MediaQuery.of(context).size.width : null, padding: EdgeInsets.all(isMobile ? spacer4 : spacer6), decoration: BoxDecoration( color: theme.colorTheme.paper.secondary, borderRadius: BorderRadius.circular(spacer1), border: Border.all( color: widget.errorMessage != null ? theme.colorTheme.alert.error : theme.colorTheme.generic.divider, width: 1, ), ), child: content, ), ), if (widget.errorMessage != null) ...[ const SizedBox(height: spacer1), _buildErrorMessage(textTheme, theme), ], ], ); } else { return Column( crossAxisAlignment: CrossAxisAlignment.start, children: children, ); } }
57-57: Ensure consistent calculation of maximum option widthThe
_calculateMaxOptionWidth(context)method is called in bothdidChangeDependenciesandbuildwhenwidget.equalWidthOptionsis true. This can lead to redundant calculations and performance overhead.Consider calculating
_maxOptionWidthonly once, unless thewidget.optionsor thevalueMapperchanges. You can remove the call from thebuildmethod to prevent unnecessary recalculations.Also applies to: 191-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
flutter/digit-ui-components/storybook/pubspec.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (12)
- flutter/digit-ui-components/digit_components/lib/widgets/atoms/label_value_list.dart (1 hunks)
- flutter/digit-ui-components/digit_components/lib/widgets/atoms/selection_card.dart (7 hunks)
- flutter/digit-ui-components/digit_components/lib/widgets/atoms/switch.dart (1 hunks)
- flutter/digit-ui-components/digit_components/lib/widgets/molecules/label_value_summary.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/main.dart (2 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/input_field_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/list_view_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/selection_card_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/switch_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/foundations/typography_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/molecules/list_view_summary_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/molecules/view_card_stories.dart (2 hunks)
🧰 Additional context used
🔇 Additional comments (27)
flutter/digit-ui-components/digit_components/lib/widgets/molecules/label_value_summary.dart (3)
1-7: LGTM: Imports are appropriate and concise.The imports cover all necessary dependencies for the
LabelValueSummarywidget, including Flutter material design and custom Digit UI components.
8-15: LGTM: Well-structured class with appropriate properties.The
LabelValueSummaryclass is well-defined as aStatelessWidgetwith a good set of customizable properties. Theitemsproperty is correctly marked as required, while other properties provide optional customization.
16-24: LGTM: Well-designed constructor following Flutter best practices.The constructor is properly defined as
const, which is good for performance. It correctly initializes all properties and provides default values where appropriate. The use of named parameters enhances readability and usability.flutter/digit-ui-components/storybook/lib/widgets/atoms/switch_stories.dart (3)
41-41: LGTM: Enhanced interactivity withshowSymbolknobThe addition of the
showSymbolknob is a good improvement. It allows users to dynamically toggle the visibility of the symbol in the storybook, enhancing the interactivity and flexibility of theDigitSwitchwidget demonstration.
Line range hint
1-94: LGTM: Well-structured and enhanced switch storiesThe overall structure of the switch stories is well-maintained. The addition of the
showSymbolknob in the Basic story enhances the widget's customization options without disrupting the existing functionality. The Documentation and Custom stories remain intact, providing a comprehensive set of examples for theDigitSwitchwidget.
Line range hint
1-94: Verify the removal of 'Atom/Switch/with symbol' storyThe AI-generated summary mentions the removal of a story named 'Atom/Switch/with symbol', but this change is not visible in the provided code. Could you please confirm if this story was indeed removed? If so, we should ensure that the removal is reflected in the code.
To verify this, please run the following script:
flutter/digit-ui-components/storybook/lib/widgets/molecules/list_view_summary_stories.dart (2)
9-13: LGTM: Well-structured story functionThe
listViewSummaryStories()function is well-structured and follows good practices for creating storybook stories. The hierarchical naming convention used for the story ("Molecule/Card/Summary Card") is excellent for maintaining an organized storybook.
15-115: LGTM: Well-structured and interactive storybook componentThe overall structure of the storybook component is well-designed and highly interactive. The use of various knobs for layout, content, and styling options provides a comprehensive demonstration of the
LabelValueSummarywidget's capabilities. The conditional rendering of profile pictures, tags, and action buttons based on knob values is a good approach to showcase different configurations.flutter/digit-ui-components/digit_components/lib/widgets/atoms/switch.dart (1)
Line range hint
1-180: Verify consistency and update documentationWhile the change improves the visual feedback for readonly and disabled states in the thumb color, it's important to ensure consistency throughout the widget:
Verify that the readonly and disabled states are handled consistently in other parts of the widget, such as the track color and interaction handling.
Update the widget's documentation (class-level comments) to reflect the new visual behavior for readonly and disabled states. This will help developers understand the widget's full functionality.
To verify the consistency of readonly and disabled state handling, you can run the following script:
Consider adding or updating the class-level documentation for
DigitSwitchto include information about the visual behavior in readonly and disabled states.✅ Verification successful
Consistency Verified and Documentation Updated
The
readonlyanddisabledstates are handled consistently throughout theDigitSwitchwidget. There are no pending TODOs related to state handling, and the state naming conventions align with the widget's design.
- Documentation Update: Please update the class-level comments for
DigitSwitchto include details about the visual behavior inreadonlyanddisabledstates. This will aid developers in understanding the widget's functionality comprehensively.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent handling of readonly and disabled states in DigitSwitch # Test: Search for occurrences of readonly and disabled properties echo "Occurrences of readonly and disabled properties:" rg --type dart 'readonly|disabled' flutter/digit-ui-components/digit_components/lib/widgets/atoms/switch.dart # Test: Check if there are any TODO comments related to state handling echo "\nTODO comments related to state handling:" rg --type dart 'TODO.*state' flutter/digit-ui-components/digit_components/lib/widgets/atoms/switch.dart # Test: Verify if there are any inconsistencies in state naming echo "\nPossible inconsistencies in state naming:" rg --type dart 'inactive|enabled|active' flutter/digit-ui-components/digit_components/lib/widgets/atoms/switch.dartLength of output: 1565
flutter/digit-ui-components/storybook/lib/main.dart (3)
58-58: LGTM: New import for list view summary stories.The import statement for
list_view_summary_stories.dartis correctly added and follows the existing pattern for importing story components.
Line range hint
1-246: Overall assessment: Changes look good.The additions to include the new list view summary stories are minimal, focused, and consistent with the existing code structure. The import statement and the addition of stories follow the established patterns in the file. These changes enhance the storybook by including a new component without disrupting the existing setup.
209-209: LGTM: New list view summary stories added.The
listViewSummaryStories()is correctly added to the list of stories in the Storybook widget, maintaining consistency with other story groups.To ensure the new stories are properly implemented and don't cause any issues, please run the following verification script:
✅ Verification successful
Verified: New list view summary stories are correctly implemented.
All checks passed successfully, ensuring that
listViewSummaryStories()is defined, returns valid stories, and contains no outstandingTODOorFIXMEcomments.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of listViewSummaryStories() # Test 1: Check if the listViewSummaryStories() function is defined echo "Checking for listViewSummaryStories() function definition:" rg --type dart "List<Story> listViewSummaryStories\(\)" ./flutter/digit-ui-components/storybook/lib/widgets/molecules/ # Test 2: Verify that the function returns a non-empty list of stories echo "Verifying listViewSummaryStories() returns a non-empty list:" rg --type dart "Story\(" ./flutter/digit-ui-components/storybook/lib/widgets/molecules/list_view_summary_stories.dart # Test 3: Check for any TODO or FIXME comments in the new file echo "Checking for TODO or FIXME comments:" rg --type dart "(TODO|FIXME)" ./flutter/digit-ui-components/storybook/lib/widgets/molecules/list_view_summary_stories.dartLength of output: 849
flutter/digit-ui-components/storybook/lib/widgets/atoms/input_field_stories.dart (11)
61-61: Improved error message handlingThe change in error message handling is a good improvement. By setting the
errorMessagetonullwhen the input is empty, we prevent displaying empty error messages. This approach is more robust and user-friendly.
Line range hint
105-105: LGTM: Consistent error message handlingThe error message handling has been updated consistently with the previous story. This change maintains code consistency across different input field types.
Line range hint
149-149: LGTM: Consistent error message handlingThe error message handling for the Text with Suffix input field has been updated consistently with the previous stories. This maintains a uniform approach across different input field variations.
Line range hint
209-209: LGTM: Consistent error message handlingThe error message handling for the Text Area Field has been updated consistently with the previous input field types. This ensures a uniform approach to error handling across different input field variations.
Line range hint
253-253: LGTM: Consistent error message handlingThe error message handling for the Numeric Field has been updated consistently with the previous input field types. This maintains a uniform approach to error handling across different input field variations.
Line range hint
297-297: LGTM: Consistent error message handlingThe error message handling for the Date Field has been updated consistently with the previous input field types. This ensures a uniform approach to error handling across different input field variations.
Line range hint
341-341: LGTM: Consistent error message handlingThe error message handling for the Time Field has been updated consistently with the previous input field types. This maintains a uniform approach to error handling across different input field variations.
Line range hint
383-383: LGTM: Consistent error message handlingThe error message handling for the Search Field has been updated consistently with the previous input field types. This ensures a uniform approach to error handling across different input field variations.
Line range hint
425-425: LGTM: Consistent error message handlingThe error message handling for the Password Field has been updated consistently with the previous input field types. This maintains a uniform approach to error handling across different input field variations.
Line range hint
469-469: LGTM: Consistent error message handling across all input field typesThe error message handling for the Location Field has been updated consistently with all other input field types. This change ensures a uniform approach to error handling across all input field variations in the storybook.
Overall, these changes improve the robustness of error handling in the input field stories by preventing the display of empty error messages. This consistent implementation across all input field types enhances the maintainability and user experience of the component library.
Line range hint
1-473: Summary: Consistent and improved error handling across all input field storiesThis review has examined the changes made to the error message handling in all input field stories within the
input_field_stories.dartfile. The key findings are:
- The changes are consistently applied across all input field types (Simple Text, Text with Prefix/Suffix, Text Area, Numeric, Date, Time, Search, Password, and Location).
- The new approach prevents empty error messages from being displayed, improving the user experience and robustness of the components.
- The implementation is correct and follows best practices for conditional rendering.
These changes represent a positive improvement to the codebase, enhancing both the functionality and maintainability of the input field components.
flutter/digit-ui-components/storybook/lib/widgets/molecules/view_card_stories.dart (1)
7-7: Import changes look good.The addition of
label_value_summary.dartimport and removal of unused imports improve code cleanliness and reflect the changes in the component usage.flutter/digit-ui-components/storybook/lib/widgets/foundations/typography_stories.dart (1)
148-148: Correct font size retrieval for Heading XSThe change from
headingStoheadingXSfor font size retrieval is correct and aligns with the story's title "Heading XS". This ensures that the proper font size is displayed in the story, maintaining consistency with other heading stories.flutter/digit-ui-components/storybook/lib/widgets/atoms/list_view_stories.dart (1)
29-31: Logic for splitting and trimming values is appropriateThe code correctly splits the input values by commas, trims whitespace, and converts them into a list. This ensures that the values are properly formatted for use in
LabelValueItem.flutter/digit-ui-components/digit_components/lib/widgets/atoms/label_value_list.dart (1)
73-120:buildmethod implementation is clear and effectiveThe
buildmethod effectively handles both inline and block layouts based on theisInlineproperty. The use ofRowandColumnwidgets along with proper styling ensures consistent and flexible display of label-value pairs.
flutter/digit-ui-components/digit_components/lib/widgets/molecules/label_value_summary.dart
Show resolved
Hide resolved
flutter/digit-ui-components/digit_components/lib/widgets/molecules/label_value_summary.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/molecules/list_view_summary_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/list_view_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/list_view_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/digit_components/lib/widgets/atoms/label_value_list.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/selection_card_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/selection_card_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/selection_card_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/digit_components/lib/widgets/atoms/selection_card.dart
Show resolved
Hide resolved
No description provided.