-
Notifications
You must be signed in to change notification settings - Fork 14
Vibing some button and auth flow changes #20
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
WalkthroughThis update standardizes button design and bottom sheet presentation across the Flutter app. It introduces enhanced button components with consistent padding, rounded corners, and style variants, and refactors authentication screens and bottom sheets to use these new patterns. Additionally, system status bar styling and bottom sheet animations are improved for visual consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Screen
participant CustomButton
participant CustomBottomSheet
User->>Screen: Navigates to Auth/Bottom Sheet screen
Screen->>CustomButton: Renders new button (with padding, style)
User->>CustomButton: Taps button
CustomButton->>Screen: Triggers callback (e.g., navigation)
User->>Screen: Triggers bottom sheet
Screen->>CustomBottomSheet: Show with blur, animation, fade-in
CustomBottomSheet->>User: Presents animated sheet with new style
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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.
Actionable comments posted: 6
♻️ Duplicate comments (1)
lib/ui/contact_list/new_group_chat_sheet.dart (1)
7-9: Import path reorganization looks consistent.The import paths follow the same pattern as other files in this PR, moving from
shared/toui/core/ui/. This appears to be part of a systematic UI component reorganization.
🧹 Nitpick comments (6)
lib/main.dart (1)
46-50: Remove redundant status bar configuration from AppBarTheme.The status bar styling in
AppBarThemeduplicates the global configuration above. Since you're setting it globally, thesystemOverlayStyleinAppBarThemeis unnecessary.appBarTheme: AppBarTheme( backgroundColor: AppColors.glitch950, // Default AppBar color for the app - systemOverlayStyle: SystemUiOverlayStyle( - statusBarColor: Colors.transparent, - statusBarIconBrightness: Brightness.light, // for Android - statusBarBrightness: Brightness.dark, // for iOS - ), ),lib/ui/contact_list/widgets/chat_list_appbar.dart (1)
16-21: Consider if local status bar override is necessary.This
SystemChrome.setSystemUIOverlayStylecall in thebuildmethod has two concerns:
- Performance: It executes on every widget rebuild, which is inefficient.
- Redundancy: Similar configuration is being set globally in
main.dart.If this AppBar needs different status bar styling than the global default, consider moving it to
initState(). If it matches the global styling, this call can be removed entirely:@override Widget build(BuildContext context) { - // Ensure status bar has light icons on this dark background - SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle( - statusBarColor: Colors.transparent, - statusBarIconBrightness: Brightness.light, // for Android - statusBarBrightness: Brightness.dark, // for iOS - )); - return ColoredBox(lib/ui/auth_flow/login_screen.dart (1)
103-123: Consider extracting shared button styling to reduce duplication.The button styling is identical across multiple auth screens. Consider creating a shared button style or using the new
CustomPaddedButtoncomponent to reduce code duplication.Example refactor using a shared style:
// Create a shared button style static final authButtonStyle = ElevatedButton.styleFrom( backgroundColor: AppColors.black, foregroundColor: AppColors.white, minimumSize: const Size(double.infinity, 56), shape: const RoundedRectangleBorder( borderRadius: BorderRadius.zero, ), ); // Then use it in the widget ElevatedButton( style: authButtonStyle, onPressed: _onContinuePressed, child: const Text('Continue', style: TextStyle(fontSize: 18)), )lib/shared/custom_padded_button.dart (1)
67-67: Consider expanding ButtonType enum for future use cases.The enum currently supports primary and secondary types. Consider if additional types (like destructive, ghost, etc.) might be needed for the broader application.
enum ButtonType { primary, secondary, // Future considerations: // destructive, // ghost, // outline }BUTTON_UPDATE_SUMMARY.md (1)
30-30: Fix grammatical issue- - Replaced fixed bottom container with padded button + - Replaced fixed bottom container with a padded button🧰 Tools
🪛 LanguageTool
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ... - Replaced fixed bottom container with padded button - **create_profile_screen.dar...(AI_EN_LECTOR_MISSING_DETERMINER_A)
lib/ui/core/ui/custom_bottom_sheet.dart (1)
26-30: Consider the unused child parameterThe
pageBuilderreturns an empty widget and the comment indicates it won't be used. While this works, it's worth documenting why this approach was chosen over using the child parameter intransitionBuilder.Consider adding a more detailed comment explaining the architectural decision:
pageBuilder: (context, animation, secondaryAnimation) { - return const SizedBox.shrink(); // This won't be used + return const SizedBox.shrink(); // Empty widget - all UI is handled in transitionBuilder for custom animations },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pubspec.lockis excluded by!**/*.lockrust_builder/cargokit/build_tool/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
BUTTON_UPDATE_SUMMARY.md(1 hunks)lib/main.dart(2 hunks)lib/shared/custom_button.dart(3 hunks)lib/shared/custom_padded_button.dart(1 hunks)lib/ui/auth_flow/create_profile_screen.dart(3 hunks)lib/ui/auth_flow/info_screen.dart(1 hunks)lib/ui/auth_flow/key_created_screen.dart(1 hunks)lib/ui/auth_flow/logged_screen.dart(1 hunks)lib/ui/auth_flow/login_screen.dart(1 hunks)lib/ui/auth_flow/welcome_screen.dart(1 hunks)lib/ui/chat/chat_screen.dart(2 hunks)lib/ui/contact_list/chat_invitation_sheet.dart(2 hunks)lib/ui/contact_list/group_chat_details_sheet.dart(2 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(1 hunks)lib/ui/contact_list/new_group_chat_sheet.dart(2 hunks)lib/ui/contact_list/search_chat_bottom_sheet.dart(1 hunks)lib/ui/contact_list/start_chat_bottom_sheet.dart(2 hunks)lib/ui/contact_list/widgets/chat_list_appbar.dart(2 hunks)lib/ui/core/ui/custom_bottom_sheet.dart(3 hunks)lib/ui/core/ui/custom_button.dart(1 hunks)
🧰 Additional context used
🪛 LanguageTool
BUTTON_UPDATE_SUMMARY.md
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ... - Replaced fixed bottom container with padded button - **create_profile_screen.dar...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🔇 Additional comments (31)
lib/ui/chat/chat_screen.dart (1)
4-4: Good addition of required import.The import is necessary for the SystemChrome functionality added below.
lib/ui/contact_list/widgets/chat_list_appbar.dart (1)
2-2: Good addition of required import.The import is necessary for the SystemChrome functionality added below.
lib/shared/custom_button.dart (3)
11-13: Excellent addition of flexible padding parameters.The new padding parameters with sensible defaults enhance the button's flexibility while maintaining consistency. The parameter names are clear and logical.
Also applies to: 19-21
43-43: Good improvement: adding rounded corners.Changing the border radius from 0 to 8 pixels improves the visual consistency and modern appearance of buttons throughout the app.
26-26: Clean implementation of conditional padding.The approach of creating a local
buttonvariable and conditionally wrapping it inPaddingis efficient and readable. This pattern avoids code duplication while providing flexible padding control.Also applies to: 53-63
lib/ui/contact_list/search_chat_bottom_sheet.dart (1)
25-27: LGTM! Enhanced bottom sheet presentation.The addition of blur background effects and customized transition duration improves the visual polish of the bottom sheet. The chosen values (8.0 blur sigma and 400ms transition) provide good visual impact without being excessive.
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
27-29: LGTM! Consistent bottom sheet styling.The addition of blur effects and transition duration maintains consistency with other bottom sheets in the app, providing a unified user experience across the interface.
lib/ui/contact_list/start_chat_bottom_sheet.dart (2)
4-4: Good refactoring: Centralized UI components.The import path change reflects better code organization by centralizing UI components under
ui/core/ui/, improving maintainability and discoverability.
34-37: Enhanced bottom sheet with transparent barrier.The addition of blur effects, transition duration, and transparent barrier color creates a modern bottom sheet presentation. The transparent barrier works well with the blur background to create a polished visual effect.
lib/ui/contact_list/chat_invitation_sheet.dart (2)
6-7: Consistent import path refactoring.The import paths have been updated to use the centralized UI components location, maintaining consistency with the codebase reorganization.
38-41: Consistent bottom sheet enhancements.The addition of blur effects, transition duration, and transparent barrier maintains visual consistency across all bottom sheets in the app, creating a unified and polished user experience.
lib/ui/contact_list/group_chat_details_sheet.dart (2)
28-31: LGTM! Enhanced bottom sheet with improved visual effects.The addition of blur background, sigma value, transition duration, and transparent barrier color enhances the user experience with a polished visual presentation. These parameters provide:
- Subtle blur effect (8.0 sigma) for better focus
- Smooth 400ms transition
- Transparent barrier for seamless interaction
10-12:✅ Verification successful
Verify the updated import paths are correct.
The import paths have been updated from
package:whitenoise/shared/topackage:whitenoise/ui/core/ui/. Ensure these new paths are valid and the components are available at these locations.
🏁 Script executed:
#!/bin/bash # Description: Verify the new import paths exist and contain the expected components # Check if the new UI component files exist fd "custom_bottom_sheet.dart" lib/ui/core/ui/ fd "custom_button.dart" lib/ui/core/ui/ fd "custom_textfield.dart" lib/ui/core/ui/ # Check if the old shared directory structure still exists fd "custom_bottom_sheet.dart" lib/shared/ fd "custom_button.dart" lib/shared/ fd "custom_textfield.dart" lib/shared/Length of output: 455
Import paths are valid and point to existing files
The new UI component imports under
lib/ui/core/ui/correctly resolve:
- lib/ui/core/ui/custom_bottom_sheet.dart
- lib/ui/core/ui/custom_button.dart
- lib/ui/core/ui/custom_textfield.dart
All referenced files exist at those locations.
lib/ui/contact_list/new_group_chat_sheet.dart (1)
23-25: LGTM! Consistent bottom sheet enhancements.The bottom sheet parameters are consistent with the pattern established in other files, providing a unified user experience across all bottom sheets with blur effects and smooth transitions.
lib/ui/auth_flow/key_created_screen.dart (3)
29-99: LGTM! Excellent UI refactor for better maintainability.The refactor from Stack-based layout to Column-based layout significantly improves code readability and maintainability while preserving all functionality. Key improvements:
- Cleaner widget tree structure with Expanded and Column
- More predictable layout behavior
- Better separation of concerns between content and bottom button
58-58: LGTM! Border radius changes align with design requirements.The change to
BorderRadius.zeroremoves rounded corners from both the container and the copy button, which aligns perfectly with the PR objective of ensuring buttons are not rounded.Also applies to: 77-77
100-120: LGTM! Improved button positioning using bottomNavigationBar.Moving the button from a Stack/Positioned layout to
bottomNavigationBaris a more idiomatic Flutter approach that:
- Provides consistent bottom button behavior
- Handles safe areas properly
- Maintains proper padding and styling
- Uses
BorderRadius.zeroconsistent with the design requirementslib/ui/auth_flow/create_profile_screen.dart (2)
61-62: LGTM! Text field styling updated to remove rounded corners.The change from
BorderRadius.circular(8)toBorderRadius.zerofor both username and bio text fields aligns with the PR objective of removing rounded styling elements. This creates visual consistency across the form.Also applies to: 84-85
95-115: LGTM! Button refactor improves consistency and maintainability.The refactor from Container+TextButton to ElevatedButton in
bottomNavigationBarprovides:
- Consistent button styling across auth flow screens
- Proper safe area handling
- Standard padding pattern (24px horizontal, 40px bottom)
- Zero border radius matching the design requirements
- Simplified widget tree structure
lib/ui/auth_flow/info_screen.dart (2)
51-89: Excellent layout refactoring that improves maintainability.The transition from
StackwithPositionedwidgets to aSafeAreaandColumnstructure is a significant improvement. This approach:
- Eliminates absolute positioning complexity
- Provides better responsive behavior
- Follows Flutter layout best practices
- Makes the code more maintainable
90-110: Consistent button styling with improved accessibility.Moving the button to
bottomNavigationBarwith properSafeAreapadding improves thumb accessibility and provides consistent spacing across devices. The switch toElevatedButtonwith zero border radius aligns with the design requirements.lib/ui/auth_flow/login_screen.dart (1)
32-102: Consistent layout improvements following the established pattern.The refactoring mirrors the improvements made in
InfoScreen, providing consistency across the authentication flow. The zero border radius on both the text field and button creates a cohesive visual design.lib/ui/auth_flow/logged_screen.dart (2)
17-114: Consistent refactoring with improved container styling.The layout improvements follow the established pattern, and the change to
BorderRadius.zeroon the progress containers maintains visual consistency with the overall design system.
53-56: Good use of const constructors for performance.The containers now use
const BoxDecorationwhich is beneficial for performance as these decorations are immutable.Also applies to: 83-86
lib/shared/custom_padded_button.dart (3)
5-19: Well-documented component with clear design intent.The documentation clearly explains the design pattern and the component provides good configurability for padding and styling.
43-51: Good disabled state handling with proper color contrast.The disabled state implementation properly handles both primary and secondary button types with appropriate alpha transparency and color contrast.
33-35:❓ Verification inconclusive
Verify flutter_screenutil responsive units usage.
Ensure that
flutter_screenutilis properly initialized in the app and that.wand.hextensions are appropriate for padding values.
🏁 Script executed:
#!/bin/bash # Verify flutter_screenutil initialization and usage patterns rg -A 5 -B 2 "ScreenUtil\.(init|ensureScreenSize)" rg -A 3 "\.w\b|\.h\b|\.r\b" --type dart | head -20Length of output: 1344
Confirm ScreenUtil Initialization
I don’t see any
ScreenUtil.init/ScreenUtil.ensureScreenSizecalls in the codebase, yet you’re using.wfor horizontal and.hfor vertical padding—which is correct so long as ScreenUtil has been initialized. Please verify that you’ve wrapped your app (e.g. inmain.dart) withScreenUtilInitor are callingScreenUtil.ensureScreenSizebefore these extensions are used, so that.wand.hresolve to valid values at runtime.• Check your app entrypoint (e.g.,
main.dart) for aScreenUtilInitwidget
• If you’re not usingScreenUtilInit, ensure you invokeScreenUtil.ensureScreenSizeearly in the app lifecyclelib/ui/core/ui/custom_button.dart (1)
1-67: Well-structured button component with good practicesThe implementation demonstrates good Flutter practices:
- Proper use of
constconstructor- Clean parameter organization with sensible defaults
- Responsive design using
flutter_screenutil- Clear separation between primary and secondary button types
lib/ui/auth_flow/welcome_screen.dart (2)
18-35: Improved layout flexibility with Expanded widgetsGood refactoring from fixed height to flexible layout using
Expandedwidgets. TheShaderMaskimplementation for gradient fade effect is clean and well-implemented.
104-113: Button styling correctly implements non-rounded designThe
ElevatedButtoncorrectly usesBorderRadius.zerowhich aligns with the PR objectives for non-rounded buttons. This is the correct implementation pattern.lib/ui/core/ui/custom_bottom_sheet.dart (1)
38-128: Excellent animation implementation with smooth transitionsThe animation architecture is well-designed:
- Separate animations for blur, barrier opacity, and slide effects
- Proper use of
AnimatedBuilderfor performance- Clean separation of concerns
- Smooth cubic easing curve
The blur effect with
BackdropFilteris implemented correctly and will provide a nice visual effect.
| // Ensure status bar has light icons on this dark background | ||
| SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle( | ||
| statusBarColor: Colors.transparent, | ||
| statusBarIconBrightness: Brightness.light, // for Android | ||
| statusBarBrightness: Brightness.dark, // for iOS | ||
| )); |
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.
🛠️ Refactor suggestion
Move status bar configuration to avoid repeated calls.
Calling SystemChrome.setSystemUIOverlayStyle in the build method causes it to execute on every widget rebuild, which is inefficient and unnecessary.
Move this configuration to initState() to execute only once:
@override
void initState() {
super.initState();
+ // Ensure status bar has light icons on this dark background
+ SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle(
+ statusBarColor: Colors.transparent,
+ statusBarIconBrightness: Brightness.light, // for Android
+ statusBarBrightness: Brightness.dark, // for iOS
+ ));
}
@override
Widget build(BuildContext context) {
- // Ensure status bar has light icons on this dark background
- SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle(
- statusBarColor: Colors.transparent,
- statusBarIconBrightness: Brightness.light, // for Android
- statusBarBrightness: Brightness.dark, // for iOS
- ));
return Scaffold(🤖 Prompt for AI Agents
In lib/ui/chat/chat_screen.dart around lines 76 to 81, the call to
SystemChrome.setSystemUIOverlayStyle is placed inside the build method causing
it to run on every rebuild. Move this call to the initState() method of the
widget so it executes only once when the widget is initialized, improving
efficiency by avoiding repeated calls during rebuilds.
| // Set status bar to light text for dark backgrounds | ||
| SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle( | ||
| statusBarColor: Colors.transparent, | ||
| statusBarIconBrightness: Brightness.light, // for Android | ||
| statusBarBrightness: Brightness.dark, // for iOS | ||
| )); |
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.
🛠️ Refactor suggestion
Move global status bar configuration to main() function.
Setting SystemChrome.setSystemUIOverlayStyle in the build method causes it to execute on every widget rebuild. Global system UI configuration should be set once during app initialization.
Move this configuration to the main() function:
Future<void> main() async {
WidgetsFlutterBinding.ensureInitialized();
+
+ // Set status bar to light text for dark backgrounds
+ SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle(
+ statusBarColor: Colors.transparent,
+ statusBarIconBrightness: Brightness.light, // for Android
+ statusBarBrightness: Brightness.dark, // for iOS
+ ));
+
//await RustLib.init();
runApp(ProviderScope(child: const MyApp()));
}Then remove the duplicate configuration from the build method:
@override
Widget build(BuildContext context, WidgetRef ref) {
final width = MediaQuery.of(context).size.width;
final router = ref.watch(routerProvider);
SystemChrome.setPreferredOrientations([
DeviceOrientation.portraitUp,
DeviceOrientation.portraitDown,
]);
-
- // Set status bar to light text for dark backgrounds
- SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle(
- statusBarColor: Colors.transparent,
- statusBarIconBrightness: Brightness.light, // for Android
- statusBarBrightness: Brightness.dark, // for iOS
- ));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set status bar to light text for dark backgrounds | |
| SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle( | |
| statusBarColor: Colors.transparent, | |
| statusBarIconBrightness: Brightness.light, // for Android | |
| statusBarBrightness: Brightness.dark, // for iOS | |
| )); | |
| // lib/main.dart | |
| Future<void> main() async { | |
| WidgetsFlutterBinding.ensureInitialized(); | |
| // Set status bar to light text for dark backgrounds | |
| SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle( | |
| statusBarColor: Colors.transparent, | |
| statusBarIconBrightness: Brightness.light, // for Android | |
| statusBarBrightness: Brightness.dark, // for iOS | |
| )); | |
| //await RustLib.init(); | |
| runApp(ProviderScope(child: const MyApp())); | |
| } |
| // Set status bar to light text for dark backgrounds | |
| SystemChrome.setSystemUIOverlayStyle(const SystemUiOverlayStyle( | |
| statusBarColor: Colors.transparent, | |
| statusBarIconBrightness: Brightness.light, // for Android | |
| statusBarBrightness: Brightness.dark, // for iOS | |
| )); | |
| @override | |
| Widget build(BuildContext context, WidgetRef ref) { | |
| final width = MediaQuery.of(context).size.width; | |
| final router = ref.watch(routerProvider); | |
| SystemChrome.setPreferredOrientations([ | |
| DeviceOrientation.portraitUp, | |
| DeviceOrientation.portraitDown, | |
| ]); | |
| // Removed the duplicate SystemChrome.setSystemUIOverlayStyle call. | |
| return MaterialApp.router( | |
| // … | |
| ); | |
| } |
🤖 Prompt for AI Agents
In lib/main.dart around lines 27 to 32, the SystemChrome.setSystemUIOverlayStyle
call is placed inside the build method, causing it to run on every widget
rebuild. Move this entire SystemChrome.setSystemUIOverlayStyle configuration to
the main() function so it runs once during app initialization, then remove it
from the build method to avoid redundant executions.
| bottomNavigationBar: SafeArea( | ||
| top: false, | ||
| child: Padding( | ||
| padding: const EdgeInsets.fromLTRB(24, 0, 24, 40), | ||
| child: ElevatedButton( | ||
| style: ElevatedButton.styleFrom( | ||
| backgroundColor: AppColors.black, | ||
| foregroundColor: AppColors.white, | ||
| minimumSize: const Size(double.infinity, 56), | ||
| shape: const RoundedRectangleBorder( | ||
| borderRadius: BorderRadius.zero, | ||
| ), | ||
| ), | ||
| onPressed: () => _onContinuePressed(context), | ||
| child: const Text( | ||
| 'Continue', | ||
| style: TextStyle(fontSize: 18), | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| ), |
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.
🛠️ Refactor suggestion
Identical button styling reinforces need for shared component.
This button styling is identical to the other auth screens, further supporting the suggestion to extract this into a shared component or use the new CustomPaddedButton.
🤖 Prompt for AI Agents
In lib/ui/auth_flow/logged_screen.dart between lines 115 and 135, the
ElevatedButton styling duplicates the same style used in other auth screens.
Refactor by replacing this button with the shared CustomPaddedButton component
to ensure consistent styling and reduce code duplication. Pass the necessary
parameters like onPressed callback and button text to the shared component
instead of defining the style inline.
| this.buttonType = ButtonType.primary, | ||
| this.horizontalPadding = 24.0, | ||
| this.bottomPadding = 24.0, | ||
| this.borderRadius = 8.0, |
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.
🛠️ Refactor suggestion
Border radius default conflicts with auth screen usage.
The default borderRadius = 8.0 conflicts with the zero border radius used throughout the auth screens. Consider either:
- Changing the default to
0.0to match current usage - Using this component in auth screens with explicit
borderRadius: 0.0 - Re-evaluating the design decision for border radius consistency
// Option 1: Change default to match current usage
this.borderRadius = 0.0,
// Option 2: Update auth screens to use this component
CustomPaddedButton(
onPressed: () => _onContinuePressed(context),
title: 'Continue',
borderRadius: 0.0, // Explicit for auth screens
)🤖 Prompt for AI Agents
In lib/shared/custom_padded_button.dart at line 18, the default borderRadius is
set to 8.0, which conflicts with the zero border radius used in auth screens. To
fix this, either change the default borderRadius value to 0.0 to align with auth
screen styling or keep the default as is and update all auth screen usages of
CustomPaddedButton to explicitly pass borderRadius: 0.0 when creating the
button.
| shape: RoundedRectangleBorder( | ||
| borderRadius: BorderRadius.circular(8.r), | ||
| ), |
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.
Critical: Button corners are rounded, contradicting PR objectives
The PR description states that "buttons were modified to ensure they are not rounded", but this implementation uses BorderRadius.circular(8.r) which creates rounded corners.
If the intention is to have non-rounded buttons as per PR objectives, apply this fix:
shape: RoundedRectangleBorder(
- borderRadius: BorderRadius.circular(8.r),
+ borderRadius: BorderRadius.zero,
),🤖 Prompt for AI Agents
In lib/ui/core/ui/custom_button.dart around lines 42 to 44, the button uses
RoundedRectangleBorder with BorderRadius.circular(8.r), which creates rounded
corners contrary to the PR objective of having non-rounded buttons. To fix this,
remove the BorderRadius.circular call or set the borderRadius to
BorderRadius.zero to ensure the button corners are not rounded.
| ### 1. Updated CustomButton Component | ||
| - **File**: `/lib/shared/custom_button.dart` | ||
| - Added default padding: 24px horizontal, 24px bottom | ||
| - Changed border radius from 0 to 8px (rounded corners) | ||
| - Added `addPadding` parameter (default: true) for flexibility | ||
| - Now matches the new design pattern automatically |
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.
Documentation contradicts PR objectives regarding rounded corners
The documentation states "Changed border radius from 0 to 8px (rounded corners)" which contradicts the PR objectives that specify buttons should not be rounded.
Please clarify the intended design and update either the implementation or documentation accordingly.
🤖 Prompt for AI Agents
In BUTTON_UPDATE_SUMMARY.md around lines 5 to 10, the documentation states that
the border radius was changed from 0 to 8px for rounded corners, which
contradicts the PR objectives specifying buttons should not be rounded. Review
the intended design for the button corners; if the design requires no rounding,
update the documentation to reflect a border radius of 0, or if rounded corners
are correct, update the PR objectives and implementation accordingly to match
the documentation.
| }, | ||
| blendMode: BlendMode.dstIn, | ||
| child: Image.asset(AssetsPaths.loginSplash, fit: BoxFit.cover), | ||
| Expanded( |
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.
@erskingardner screenHeigth var can be removed as it is no longer used.
| ), | ||
| ), | ||
| ], | ||
| ), |
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.
@erskingardner After the nsec login, you should be redirected to the LoggedInScreen instead of the chat screen, because the continue button in the LoggedInScreen already goes to the chat screen. but I think it doesn't make sense to fix it on this branch, we can fix it later by working on the Go router.
| ); | ||
| } | ||
| } | ||
| } |
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.
I don't know if I should add this here, but I think such sharp lines are a bit eye straining. Just for your and designers' information.
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.
For now, let's stick with the designs as they are (other than moving those buttons up off the bottom). I think we'll be able to test with real users soon and we might end up softening things.
| borderRadius: BorderRadius.zero, | ||
| ), | ||
| ), | ||
| onPressed: _onContinuePressed, |
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.
@erskingardner Right now the snackbar appears on the wrong position, it should be at the bottom of the screen, but in the real scenario the snackbar will not appear. So we can remove it completely if it's okay for you.
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.
What's the snackbar?
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.
When you don't enter nsec, it says 'Please enter something' at the bottom of the screen.
|
I hope you pulled from master so you get the format changes merged yesterday in #17 @erskingardner |
Quwaysim
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.
Could we merge the shared widgets together from bib/shared and lib/core/ui? We have two instances of CustomTextField and 3 instances of CustomButton.
Used Goose this morning to vibe some clean up on button location (not pinned to the bottom of the page any more) and make sure the ones I was seeing weren't rounded.
Summary by CodeRabbit