Updated button, alert card and back button stories #137
Conversation
WalkthroughThe pull request introduces various modifications across several Dart files within the Flutter project. Key changes include updates to the Changes
Suggested reviewers
Poem
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: 10
🧹 Outside diff range and nitpick comments (11)
flutter/digit-ui-components/storybook/lib/widgets/atoms/back_button_stories.dart (2)
20-20: Consider a more descriptive name for the storyThe story name has been changed from "Double arrow" to "Backlink 1". While this new name is more consistent with the other stories, it loses some descriptive information about the button's appearance. Consider a name that combines both aspects, such as "Backlink 1 - Double Arrow".
The addition of the
digitBackButtonThemeDataconfiguration is a good improvement. It allows for more customization and responsiveness:
- The icon size adapts to the screen width, which is good for responsiveness.
- The color is set using the theme, which ensures consistency with the overall design.
Also applies to: 26-36
Line range hint
43-58: Consider a more descriptive name and potential refactoringSimilar to the previous story, the name has been changed from "Arrow with circle" to "Backlink 2". Consider a name that retains some description of the button's appearance, such as "Backlink 2 - Circled Arrow".
The theme configuration is very similar to the first story, with only the icon changing. To reduce code duplication, consider extracting the common parts of the theme configuration into a separate function or constant. This would make the stories more maintainable and easier to update in the future.
Here's a potential refactor to reduce duplication:
DigitBackButtonThemeData getCommonTheme(BuildContext context) { return const DigitBackButtonThemeData().copyWith( context: context, backDigitButtonIcon: Icon( // Icon can be passed as a parameter if needed Icons.arrow_circle_left_outlined, size: MediaQuery.of(context).size.width < 500 ? Theme.of(context).spacerTheme.spacer5 : Theme.of(context).spacerTheme.spacer6, color: Theme.of(context).colorTheme.primary.primary2, ), ); } // In the story: digitBackButtonThemeData: getCommonTheme(context),This approach would make it easier to maintain consistent styling across different back button variants.
flutter/digit-ui-components/storybook/lib/main.dart (1)
79-79: LGTM! Consider dynamic theming for better user experience.The addition of
themeMode: ThemeMode.lightensures a consistent light theme across the app. However, for improved user experience, consider implementing dynamic theming that respects the user's system preferences.You could use
ThemeMode.systeminstead, which automatically adapts to the user's system settings:themeMode: ThemeMode.system,flutter/digit-ui-components/storybook/lib/widgets/atoms/button_stories.dart (5)
22-45: Approved: Enhanced interactivity and clarity for Primary Button storyThe changes to the Primary Button story are well-implemented:
- The new option knob for disabled states enhances testing capabilities.
- The updated label to "Primary Button" and size option to "Size of button" improve clarity.
- The
isDisabledproperty is correctly set based on user selection.These improvements make the story more interactive and user-friendly.
Consider simplifying the
isDisabledlogic:- bool isDisabled = false; - if (fieldState == 'disabled') { - isDisabled = true; - } + bool isDisabled = fieldState == 'disabled';This change would make the code more concise without altering its functionality.
80-107: Approved: Consistent improvements for Secondary Button storyThe changes to the Secondary Button story mirror those of the Primary Button story, maintaining consistency across button types:
- The new option knob for disabled states enhances testing capabilities.
- The updated label to "Secondary Button" improves clarity.
- The
isDisabledproperty is correctly set based on user selection.As with the Primary Button story, consider simplifying the
isDisabledlogic:- bool isDisabled = false; - if (fieldState == 'disabled') { - isDisabled = true; - } + bool isDisabled = fieldState == 'disabled';This change would make the code more concise without altering its functionality.
150-177: Approved: Consistent improvements for Tertiary Button story with minor inconsistencyThe changes to the Tertiary Button story are consistent with the Primary and Secondary Button stories:
The new option knob for disabled states enhances testing capabilities.
The updated label to "Tertiary Button" improves clarity.
The
isDisabledproperty is correctly set based on user selection.Consider simplifying the
isDisabledlogic:- bool isDisabled = false; - if (fieldState == 'disabled') { - isDisabled = true; - } + bool isDisabled = fieldState == 'disabled';
- Update the size option label to match the other stories:
- label: 'Size of DigitButton', + label: 'Size of button',These changes would improve consistency across all button stories.
220-246: Approved: Consistent improvements for Link Button story with minor inconsistenciesThe changes to the Link Button story are consistent with the other button stories:
- The new option knob for disabled states enhances testing capabilities.
- The
isDisabledproperty is correctly set based on user selection.Consider the following improvements for consistency:
- Simplify the
isDisabledlogic:- bool isDisabled = false; - if (fieldState == 'disabled') { - isDisabled = true; - } + bool isDisabled = fieldState == 'disabled';
- Update the initial label to match the pattern of other stories:
- initial: 'link' + initial: 'Link Button'
- Update the size option label to match the other stories:
- label: 'Size of DigitButton', + label: 'Size of button',These changes would improve consistency across all button stories.
Line range hint
1-285: Overall improvements enhance button stories functionality and consistencyThe changes made to the
buttonStories()function and its individual stories significantly improve the overall functionality and user experience:
- The addition of disabled state options across all button types enhances testing capabilities.
- Updated labels improve clarity and consistency.
- The changes maintain a consistent structure across different button types.
These improvements will make it easier for developers to interact with and test various button states and sizes.
To further enhance consistency, consider addressing the minor inconsistencies in labels and simplifying the
isDisabledlogic as suggested in the individual story comments. These small adjustments will result in a more uniform and maintainable codebase.flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart (3)
60-74: Consider using different images for varietyAll stories use the same image URL multiple times. To enhance the visual appeal and demonstrate versatility, consider using different images or varying the number of images displayed.
This can provide a better user experience when showcasing the component's capabilities.
Also applies to: 135-149, 210-224, 285-299, 360-374
173-183: Ensure consistent indentation and formattingThere are inconsistencies in indentation starting from line 173 to line 183. Proper formatting enhances readability and maintainability.
Apply Dart's code formatting guidelines to these lines.
248-258: Correct the initial description in 'Warning' storyThe initial description in the 'Warning' story is the same as in other stories. Consider customizing it to better reflect the warning context.
Example:
- initial: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit...', + initial: 'Please be cautious! This action may have unintended consequences.',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
flutter/digit-ui-components/storybook/assets/localized_data.jsonis excluded by!**/*.jsonflutter/digit-ui-components/storybook/pubspec.lockis excluded by!**/*.lock,!**/*.lockflutter/digit-ui-components/storybook/pubspec_overrides.yamlis excluded by!**/*.yaml
📒 Files selected for processing (5)
- flutter/digit-ui-components/storybook/lib/iframe/iframe_widget.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/main.dart (2 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart (1 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/back_button_stories.dart (2 hunks)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/button_stories.dart (5 hunks)
🧰 Additional context used
🔇 Additional comments (5)
flutter/digit-ui-components/storybook/lib/main.dart (1)
172-175: LGTM! Verify new story functions.The addition of
infoCardStories()and the replacement ofDigitButtonStories()withbuttonStories()align with the PR objectives. These changes should improve the organization and clarity of the storybook.Please ensure that:
- The
infoCardStories()function is properly implemented and contains the expected stories for info cards.- The
buttonStories()function includes all the stories previously inDigitButtonStories()and any new button stories mentioned in the PR objectives.You can verify this by running the following script:
✅ Verification successful
Verification Successful: New story functions are correctly implemented.
The addition of
infoCardStories()and the replacement ofDigitButtonStories()withbuttonStories()have been verified. These changes enhance the organization and clarity of the Storybook.
infoCardStories()is properly implemented and contains the expected stories for info cards.buttonStories()is properly implemented and includes all stories previously inDigitButtonStories().🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of new story functions # Test 1: Check if infoCardStories() is implemented echo "Checking infoCardStories() implementation:" rg --type dart "function infoCardStories\(\)" ./flutter/digit-ui-components/storybook/ # Test 2: Check if buttonStories() is implemented and contains expected content echo "Checking buttonStories() implementation:" rg --type dart "function buttonStories\(\)" ./flutter/digit-ui-components/storybook/ # Test 3: Verify that DigitButtonStories is no longer used echo "Verifying DigitButtonStories is not used:" rg --type dart "DigitButtonStories" ./flutter/digit-ui-components/storybook/Length of output: 531
Script:
#!/bin/bash # Description: Re-verify the implementation of new story functions with refined search patterns # Test 1: Check if infoCardStories() exists using a broader search pattern echo "Re-checking infoCardStories() implementation with broader pattern:" rg --type dart "infoCardStories" ./flutter/digit-ui-components/storybook/ # Test 2: Check if buttonStories() exists using a broader search pattern echo "Re-checking buttonStories() implementation with broader pattern:" rg --type dart "buttonStories" ./flutter/digit-ui-components/storybook/ # Test 3: Verify that DigitButtonStories is not used anywhere echo "Re-verifying DigitButtonStories is not used:" rg --type dart "DigitButtonStories" ./flutter/digit-ui-components/storybook/Length of output: 994
flutter/digit-ui-components/storybook/lib/widgets/atoms/button_stories.dart (2)
8-8: Approved: Function name change improves consistencyThe change from
DigitButtonStoriestobuttonStoriesis a good improvement. It makes the function name more concise and aligns with common Dart naming conventions, enhancing overall code readability.
Line range hint
11-17: Approved: Story name update and documentation approachThe change to use 'Buttons' in plural form in the story name is appropriate, as it better represents the multiple button types covered. The use of an
IframeWidgetto display external documentation is a good practice, allowing for easy updates and maintenance of the documentation separate from the code.flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart (2)
38-45: Verify 'Widget Alignment' option valuesThe 'Widget Alignment' options assign
trueto "Vertical" andfalseto "Horizontal". This might be counterintuitive since "inline" typically refers to horizontal alignment. Ensure that these values correctly reflect the intended behavior of theinlineparameter inInfoCard.Please confirm whether
inline: trueresults in vertical alignment or if the values should be swapped.
113-121: Consistent 'Widget Alignment' option values across storiesSimilar to the earlier verification, ensure that the 'Widget Alignment' option values are consistent and correctly mapped in all stories. The mapping should accurately reflect whether
inline: truecorresponds to vertical or horizontal alignment.Please confirm the mapping in the following line ranges:
- Lines 113-121
- Lines 188-196
- Lines 263-271
- Lines 338-346
Adjust the option values if necessary to maintain consistent behavior across all stories.
Also applies to: 188-196, 263-271, 338-346
flutter/digit-ui-components/storybook/lib/widgets/atoms/back_button_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Outdated
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Outdated
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Outdated
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Show resolved
Hide resolved
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Outdated
Show resolved
Hide resolved
|
@coderabbitai can you approve the pr |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart (3)
12-16: Update documentation URLThe URL provided in the 'Documentation' story appears to be a placeholder or potentially outdated. Please ensure that it points to the correct and up-to-date documentation for the
InfoCardcomponent.
60-66: 🛠️ Refactor suggestionRefactor repeated Image.network calls
The current implementation repeats the
Image.networkcall multiple times. Consider refactoring this to improve code maintainability:if (showAdditionalWidget) ...List.generate( 5, (_) => Image.network( 'https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcQIGMLufj86aep95KwMzr3U0QShg7oxdAG8gBPJ9ALIFQ&s', ), ),This approach reduces code duplication and makes it easier to adjust the number of images in the future.
319-320: 🛠️ Refactor suggestionEnhance customization options for the Custom story
The Custom story currently has a hardcoded
InfoType.info. To make this story truly customizable, consider adding a knob to select theInfoType. Additionally, update the initial title to reflect that this is a custom story.- title: context.knobs.text(label: "Title", initial: 'Info'), + title: context.knobs.text(label: "Title", initial: 'Custom'), - type: InfoType.info, + type: context.knobs.options( + label: "Info Type", + options: InfoType.values.map((type) => Option(label: type.name, value: type)).toList(), + initial: InfoType.info, + ),This change allows users to fully customize the alert type in the Custom story.
flutter/digit-ui-components/storybook/lib/widgets/atoms/Info_card_stories.dart
Show resolved
Hide resolved
|
@naveen-egov: Sure, I have approved the PR. ✅ Actions performedComments resolved and changes approved. |
No description provided.