-
-
Notifications
You must be signed in to change notification settings - Fork 45
-rounded button issue resolved #2932
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
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive update across multiple layout files and Java classes, focusing on replacing custom The modifications involve systematically updating button implementations across various fragments and screens, such as login, delivery details, learning progress, and phone verification interfaces. Key changes include replacing custom button attributes with Material Design attributes, adjusting button heights from fixed dimensions to These changes suggest a move towards more consistent UI design principles and potentially leveraging the built-in capabilities of Material Design components. Sequence DiagramsequenceDiagram
participant UI as User Interface
participant Layout as Layout XML
participant MaterialButton as MaterialButton/Button
UI->>Layout: Render Screen
Layout->>MaterialButton: Initialize Button
MaterialButton-->>Layout: Apply Styling
MaterialButton->>UI: Display Interactive Button
The sequence diagram illustrates the simplified process of rendering buttons after the component replacement, showing how the new button types are initialized and displayed in the user interface. 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 (
|
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: 9
🧹 Nitpick comments (7)
app/res/layout/screen_connect_primary_phone.xml (1)
158-165: Consider using wrap_content for widthWhile the migration to MaterialButton is correct, consider using
wrap_contentfor the width instead of a fixed120dpto better handle different text lengths and translations.- android:layout_width="120dp" + android:layout_width="wrap_content"app/res/layout/screen_connect_phone_verify.xml (1)
156-164: Consider using wrap_content for button widthsWhile the migration to MaterialButton and styling is correct, consider using
wrap_contentfor the width of all buttons to better handle different text lengths and translations.- android:layout_width="150dp" + android:layout_width="wrap_content"Also applies to: 166-173, 175-182
app/res/layout/select_install_mode_fragment.xml (2)
43-50: Consider using standard margin attributesWhile the MaterialButton implementation is correct, consider using the standard margin attribute instead of separate horizontal margins for consistency:
- android:layout_marginHorizontal="30dp" + android:layout_margin="@dimen/content_min_margin"
Line range hint
43-198: Consider creating a custom MaterialButton style for consistencyTo ensure consistent button styling across the app, consider creating a custom style that extends MaterialButton.Style with your common attributes (margins, padding, etc.). This would help maintain consistency and make future updates easier.
Example style definition:
<style name="ConnectMaterialButton" parent="Widget.MaterialComponents.Button"> <item name="android:layout_width">wrap_content</item> <item name="android:layout_height">wrap_content</item> <item name="android:layout_margin">@dimen/content_min_margin</item> </style>app/res/layout/screen_login.xml (1)
85-93: Consider adding style attributes for consistency.The MaterialButton implementation looks good, but consider adding the following attributes for better consistency with Material Design:
android:textAllCapsapp:cornerRadiusapp:elevation<com.google.android.material.button.MaterialButton android:id="@+id/connect_login_button" android:layout_width="fill_parent" android:layout_height="wrap_content" android:layout_gravity="bottom" android:paddingTop="@dimen/content_start" android:paddingBottom="@dimen/content_start" android:text="@string/connect_button_logged_in" - android:visibility="gone"/> + android:visibility="gone" + android:textAllCaps="false" + app:cornerRadius="8dp" + app:elevation="2dp"/>app/res/layout/fragment_connect_learning_progress.xml (1)
317-322: Ensure consistent styling across MaterialButtons.Both buttons should maintain consistent styling with other MaterialButtons in the app.
<com.google.android.material.button.MaterialButton android:id="@+id/connect_learning_review_button" android:layout_width="match_parent" android:layout_height="wrap_content" android:layout_marginHorizontal="16dp" android:layout_marginVertical="8dp" + android:textAllCaps="false" + app:cornerRadius="8dp" + app:elevation="2dp" android:visibility="visible" tools:text="@string/connect_learn_review" /> <com.google.android.material.button.MaterialButton android:id="@+id/connect_learning_button" android:layout_width="match_parent" android:layout_height="wrap_content" android:layout_marginHorizontal="16dp" android:layout_marginVertical="8dp" + android:textAllCaps="false" + app:cornerRadius="8dp" + app:elevation="2dp" tools:text="Button" />Also applies to: 326-332
app/res/layout/temp_login.xml (1)
Line range hint
85-93: Consider defining a global style for MaterialButtons.To maintain consistency and reduce duplication, consider creating a base style for MaterialButtons in
styles.xml:<!-- styles.xml --> <style name="Widget.App.Button" parent="Widget.MaterialComponents.Button"> <item name="android:textAllCaps">false</item> <item name="cornerRadius">8dp</item> <item name="elevation">2dp</item> <item name="android:fontFamily">@font/roboto_medium</item> </style>Then apply it to all MaterialButtons using
style="@style/Widget.App.Button". This will:
- Ensure consistent styling across the app
- Make it easier to maintain and update button styles
- Follow Material Design best practices
Also applies to: 169-180, 317-322, 326-332, 271-280
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
app/res/drawable/ic_connect_arrow_forward_24px.xml(1 hunks)app/res/layout-land/home_screen.xml(0 hunks)app/res/layout/dialog_payment_confirmation.xml(1 hunks)app/res/layout/fragment_connect_delivery_details.xml(1 hunks)app/res/layout/fragment_connect_delivery_progress.xml(0 hunks)app/res/layout/fragment_connect_job_intro.xml(1 hunks)app/res/layout/fragment_connect_learning_progress.xml(2 hunks)app/res/layout/fragment_connect_progress_delivery.xml(1 hunks)app/res/layout/fragment_phone_available_bottom_sheet.xml(1 hunks)app/res/layout/fragment_recovery_code.xml(1 hunks)app/res/layout/fragment_secondary_phone_number.xml(1 hunks)app/res/layout/fragment_signup.xml(2 hunks)app/res/layout/home_screen.xml(0 hunks)app/res/layout/screen_connect_message.xml(1 hunks)app/res/layout/screen_connect_password_verify.xml(1 hunks)app/res/layout/screen_connect_phone_verify.xml(1 hunks)app/res/layout/screen_connect_primary_phone.xml(1 hunks)app/res/layout/screen_connect_user_deactivate_otp_verify.xml(1 hunks)app/res/layout/screen_connect_verify.xml(2 hunks)app/res/layout/screen_login.xml(1 hunks)app/res/layout/select_install_mode_fragment.xml(1 hunks)app/res/layout/temp_login.xml(1 hunks)app/src/org/commcare/activities/LoginActivityUiController.java(1 hunks)app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java(1 hunks)app/src/org/commcare/fragments/connect/ConnectDeliveryProgressDeliveryFragment.java(3 hunks)app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java(1 hunks)
💤 Files with no reviewable changes (3)
- app/res/layout/fragment_connect_delivery_progress.xml
- app/res/layout/home_screen.xml
- app/res/layout-land/home_screen.xml
✅ Files skipped from review due to trivial changes (1)
- app/res/drawable/ic_connect_arrow_forward_24px.xml
🔇 Additional comments (17)
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java (1)
115-115: Verify button styling after RoundedButton replacement.The change from
RoundedButtontoButtonis part of the broader transition to standard button components. Please ensure that the button's appearance still matches the Figma design specifications for the Connect feature.app/src/org/commcare/fragments/connect/ConnectDeliveryProgressDeliveryFragment.java (1)
38-38: Verify button styling after RoundedButton replacement.The changes from
RoundedButtontoButtonforlaunchButton,btnSync, andreviewButtonare part of the broader transition to standard button components. Please ensure that all buttons' appearances still match the Figma design specifications for the Connect feature.Also applies to: 70-70, 79-79
app/src/org/commcare/activities/LoginActivityUiController.java (1)
73-73: Verify button styling after RoundedButton replacement.The change from
RoundedButtontoButtonforconnectLoginButtonis part of the broader transition to standard button components. Please ensure that the button's appearance still matches the Figma design specifications for the Connect feature.app/res/layout/screen_connect_message.xml (1)
35-41: LGTM! Good improvements to button styling and accessibility.The transition from
RoundedButtontoMaterialButtonwithwrap_contentheight improves accessibility and follows Material Design guidelines.Also applies to: 43-53
app/res/layout/screen_connect_password_verify.xml (1)
117-124: LGTM!The MaterialButton implementation is correct with proper layout parameters and alignment.
app/res/layout/screen_connect_user_deactivate_otp_verify.xml (2)
131-137: LGTM! Proper migration to MaterialButton with negative styleThe conversion from RoundedButton to MaterialButton with NegativeButtonStyle is appropriate for the resend button, maintaining the intended UI hierarchy.
139-145: LGTM! Clean MaterialButton implementationThe verify button implementation follows Material Design guidelines with appropriate layout parameters.
app/res/layout/fragment_recovery_code.xml (1)
171-183: LGTM! Well-implemented MaterialButton conversionThe migration to MaterialButton is well done with:
- Appropriate use of NegativeButtonStyle for the forgot button
- Proper icon integration using drawableRight
- Flexible width using wrap_content
Also applies to: 185-198
app/res/layout/fragment_secondary_phone_number.xml (2)
176-185: LGTM! Back button implementation follows Material Design guidelines.The transition from
RoundedButtontoMaterialButtonwithNegativeButtonStyleis appropriate for a secondary action button.
187-194: LGTM! Continue button implementation follows Material Design guidelines.The transition to
MaterialButtonwith right arrow drawable is well-implemented and maintains the visual hierarchy.app/res/layout/fragment_connect_progress_delivery.xml (3)
156-166: LGTM! Sync button implementation follows Material Design guidelines.The sync button's styling with right drawable and padding is appropriate for its action.
171-183: LGTM! Review button implementation follows Material Design guidelines.The review button's styling and font family are consistent with Material Design principles.
185-197: LGTM! Progress button implementation follows Material Design guidelines.The progress button's styling and font family maintain consistency with other buttons.
app/res/layout/fragment_connect_delivery_details.xml (1)
188-201: LGTM! Delivery button implementation follows Material Design guidelines.The button styling with background tint, icon, and font family creates a cohesive look with the app's theme.
app/res/layout/fragment_signup.xml (2)
219-229: LGTM! Continue button implementation follows Material Design guidelines.The primary action button with right arrow drawable maintains proper visual hierarchy.
265-271: LGTM! Recover button implementation follows Material Design guidelines.The secondary action button with
NegativeButtonStyleprovides appropriate visual contrast.app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java (1)
68-68: LGTM! Button type change is consistent with layout changes.The change from
RoundedButtonto standardButtonaligns with the migration to Material Design components.
| app:roundButtonBackgroundColor="@color/connect_blue_color" | ||
| app:roundButtonTextColor="@color/white" | ||
| app:roundButtonTextSize="6.5sp" /> | ||
| android:text="Back" /> |
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 hardcoded text to string resources.
The "Back" text should be moved to string resources for proper localization support.
Apply this change:
- android:text="Back" />
+ android:text="@string/connect_back_button" />- android:text="Back"
+ android:text="@string/connect_back_button"Also applies to: 52-52
| <com.google.android.material.button.MaterialButton | ||
| style="@style/NegativeButtonStyle" | ||
| android:id="@+id/ri_no" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="35dp" | ||
| android:layout_marginEnd="16dp" | ||
| android:text="@string/connect_payment_dialog_no" | ||
| app:roundButtonBackgroundColor="@color/white" | ||
| app:roundButtonBorderVisible="true" | ||
| app:roundButtonBorderWidth="0.5dp" | ||
| app:roundButtonTextColor="@color/black" | ||
| app:roundButtonTextSize="5dp" /> | ||
| android:textSize="5dp" /> |
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.
Fix text size and height constraints for better accessibility.
The text size of 5dp is extremely small and uses incorrect units. For text sizes, always use sp units to respect system font scaling settings.
Apply this fix:
- android:layout_height="35dp"
- android:textSize="5dp"
+ android:layout_height="wrap_content"
+ android:textSize="14sp"📝 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.
| <com.google.android.material.button.MaterialButton | |
| style="@style/NegativeButtonStyle" | |
| android:id="@+id/ri_no" | |
| android:layout_width="wrap_content" | |
| android:layout_height="35dp" | |
| android:layout_marginEnd="16dp" | |
| android:text="@string/connect_payment_dialog_no" | |
| app:roundButtonBackgroundColor="@color/white" | |
| app:roundButtonBorderVisible="true" | |
| app:roundButtonBorderWidth="0.5dp" | |
| app:roundButtonTextColor="@color/black" | |
| app:roundButtonTextSize="5dp" /> | |
| android:textSize="5dp" /> | |
| <com.google.android.material.button.MaterialButton | |
| style="@style/NegativeButtonStyle" | |
| android:id="@+id/ri_no" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_marginEnd="16dp" | |
| android:text="@string/connect_payment_dialog_no" | |
| android:textSize="14sp" /> |
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/ri_yes" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="35dp" | ||
| android:text="@string/connect_payment_dialog_yes" | ||
| app:roundButtonBackgroundColor="@color/connect_blue_color" | ||
| app:roundButtonBorderVisible="false" | ||
| app:roundButtonBorderWidth="0.5dp" | ||
| app:roundButtonTextColor="@color/white" | ||
| app:roundButtonTextSize="5dp" /> | ||
| android:textColor="@color/white" | ||
| android:textSize="5sp" /> |
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.
Maintain consistent text size across buttons.
The "Yes" button uses a different text size unit (sp) but the same small value. Both buttons should use the same text size for visual consistency.
Apply this fix:
- android:layout_height="35dp"
- android:textSize="5sp"
+ android:layout_height="wrap_content"
+ android:textSize="14sp"📝 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.
| <com.google.android.material.button.MaterialButton | |
| android:id="@+id/ri_yes" | |
| android:layout_width="wrap_content" | |
| android:layout_height="35dp" | |
| android:text="@string/connect_payment_dialog_yes" | |
| app:roundButtonBackgroundColor="@color/connect_blue_color" | |
| app:roundButtonBorderVisible="false" | |
| app:roundButtonBorderWidth="0.5dp" | |
| app:roundButtonTextColor="@color/white" | |
| app:roundButtonTextSize="5dp" /> | |
| android:textColor="@color/white" | |
| android:textSize="5sp" /> | |
| <com.google.android.material.button.MaterialButton | |
| android:id="@+id/ri_yes" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:text="@string/connect_payment_dialog_yes" | |
| android:textColor="@color/white" | |
| android:textSize="14sp" /> |
| android:layout_alignParentRight="true" | ||
| android:layout_marginStart="16dp" | ||
| android:layout_marginTop="16dp" | ||
| android:layout_marginEnd="16dp" | ||
| android:layout_marginBottom="16dp" | ||
| android:gravity="start|center" | ||
| android:text="Recover" | ||
| app:roundButtonBackgroundColor="@color/connect_blue_color" | ||
| app:roundButtonBorderRadius="@dimen/dp48" | ||
| app:roundButtonIcon="@drawable/connect_right_arrow" | ||
| app:roundButtonIconLeftAlign="false" | ||
| app:roundButtonTextColor="@color/white" /> | ||
| android:drawableRight="@drawable/connect_right_arrow" /> |
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
Replace deprecated layout alignment and use string resources.
layout_alignParentRightis deprecated, uselayout_alignParentEnd- Replace hardcoded "Recover" text with a string resource
- android:layout_alignParentRight="true"
+ android:layout_alignParentEnd="true"
- android:text="Recover"
+ android:text="@string/connect_button_recover"📝 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.
| android:layout_alignParentRight="true" | |
| android:layout_marginStart="16dp" | |
| android:layout_marginTop="16dp" | |
| android:layout_marginEnd="16dp" | |
| android:layout_marginBottom="16dp" | |
| android:gravity="start|center" | |
| android:text="Recover" | |
| app:roundButtonBackgroundColor="@color/connect_blue_color" | |
| app:roundButtonBorderRadius="@dimen/dp48" | |
| app:roundButtonIcon="@drawable/connect_right_arrow" | |
| app:roundButtonIconLeftAlign="false" | |
| app:roundButtonTextColor="@color/white" /> | |
| android:drawableRight="@drawable/connect_right_arrow" /> | |
| android:layout_alignParentEnd="true" | |
| android:layout_marginStart="16dp" | |
| android:layout_marginTop="16dp" | |
| android:layout_marginEnd="16dp" | |
| android:layout_marginBottom="16dp" | |
| android:gravity="start|center" | |
| android:text="@string/connect_button_recover" | |
| android:drawableRight="@drawable/connect_right_arrow" /> |
| app:roundButtonBorderColor="@color/border_grey" | ||
| app:roundButtonBorderVisible="true" | ||
| app:roundButtonTextColor="@color/connect_blue_color" /> | ||
| android:text="Back" /> |
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
Use string resources instead of hardcoded text.
Replace hardcoded "Back" text with a string resource for better localization support.
- android:text="Back"
+ android:text="@string/connect_button_back"📝 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.
| android:text="Back" /> | |
| android:text="@string/connect_button_back" /> |
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/connect_verify_fingerprint_button" | ||
| android:layout_width="fill_parent" | ||
| android:layout_height="45dp" | ||
| android:layout_gravity="center" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginHorizontal="20dp" | ||
| android:layout_marginTop="16dp" | ||
| android:text="@string/connect_verify_configure_fingerprint" | ||
| app:roundButtonBackgroundColor="@color/connect_blue_color" | ||
| app:roundButtonTextColor="@color/white" | ||
| app:roundButtonTextSize="6sp" /> | ||
| android:text="@string/connect_verify_configure_fingerprint" /> |
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
Replace deprecated layout_width value.
Replace deprecated fill_parent with match_parent for both buttons.
- android:layout_width="fill_parent"
+ android:layout_width="match_parent"Also applies to: 103-109
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/connect_job_intro_start_button" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="40dp" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" | ||
| app:roundButtonTextSize="5sp" | ||
| android:layout_marginStart="20dp" | ||
| android:gravity="center" | ||
| android:text="Continue" | ||
| app:roundButtonIcon="@drawable/connect_right_arrow" | ||
| app:roundButtonIconTintColor="@color/connect_blue_color" | ||
| app:roundButtonTextColor="@color/connect_blue_color" /> | ||
| android:drawableRight= "@drawable/ic_connect_arrow_forward_24px" | ||
| android:backgroundTint= "@color/white" | ||
| android:textColor= "@color/cc_brand_color" /> |
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
Use string resources and style attributes.
- Replace hardcoded "Continue" text with a string resource
- Consider moving colors to a style for better maintainability
- android:text="Continue"
+ android:text="@string/connect_button_continue"
- android:backgroundTint="@color/white"
- android:textColor="@color/cc_brand_color"
+ style="@style/Widget.Connect.Button.White"Committable suggestion skipped: line range outside the PR's diff.
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/btnSync" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="40dp" | ||
| android:layout_height="wrap_content" | ||
| android:text="@string/sync" | ||
| app:layout_constraintBottom_toBottomOf="parent" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent" | ||
| app:roundButtonBackgroundColor="@color/connect_dark_blue_color" | ||
| app:roundButtonFontFamily="@font/roboto_medium" | ||
| app:roundButtonIcon="@drawable/ic_connect_directory_sync" | ||
| app:roundButtonIconLeftAlign="false" | ||
| app:roundButtonIconPadding="5dp" | ||
| app:roundButtonTextColor="@color/white" | ||
| app:roundButtonTextSize="5sp" /> | ||
| android:fontFamily= "@font/roboto_medium" | ||
| app:iconSize="24dp" | ||
| android:drawableRight="@drawable/ic_connect_directory_sync" | ||
| /> |
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
Replace deprecated drawableRight with Material's app:icon.
Use Material Design's icon support instead of the deprecated android:drawableRight.
<com.google.android.material.button.MaterialButton
android:id="@+id/btnSync"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@string/sync"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="parent"
android:fontFamily="@font/roboto_medium"
app:iconSize="24dp"
- android:drawableRight="@drawable/ic_connect_directory_sync"
+ app:icon="@drawable/ic_connect_directory_sync"
+ app:iconGravity="end"
+ app:iconTint="?attr/colorOnPrimary"/>📝 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.
| <com.google.android.material.button.MaterialButton | |
| android:id="@+id/btnSync" | |
| android:layout_width="wrap_content" | |
| android:layout_height="40dp" | |
| android:layout_height="wrap_content" | |
| android:text="@string/sync" | |
| app:layout_constraintBottom_toBottomOf="parent" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintTop_toTopOf="parent" | |
| app:roundButtonBackgroundColor="@color/connect_dark_blue_color" | |
| app:roundButtonFontFamily="@font/roboto_medium" | |
| app:roundButtonIcon="@drawable/ic_connect_directory_sync" | |
| app:roundButtonIconLeftAlign="false" | |
| app:roundButtonIconPadding="5dp" | |
| app:roundButtonTextColor="@color/white" | |
| app:roundButtonTextSize="5sp" /> | |
| android:fontFamily= "@font/roboto_medium" | |
| app:iconSize="24dp" | |
| android:drawableRight="@drawable/ic_connect_directory_sync" | |
| /> | |
| <com.google.android.material.button.MaterialButton | |
| android:id="@+id/btnSync" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:text="@string/sync" | |
| app:layout_constraintBottom_toBottomOf="parent" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintTop_toTopOf="parent" | |
| android:fontFamily="@font/roboto_medium" | |
| app:iconSize="24dp" | |
| app:icon="@drawable/ic_connect_directory_sync" | |
| app:iconGravity="end" | |
| app:iconTint="?attr/colorOnPrimary"/> |
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/login_button" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="end" | ||
| android:layout_marginVertical="24dp" | ||
| android:text="Login with Connect ID" | ||
| RectangleButtonWithText:roundButtonBackgroundColor="@color/connect_blue_color" | ||
| RectangleButtonWithText:roundButtonIcon="@drawable/connect_right_arrow" | ||
| RectangleButtonWithText:roundButtonTextColor="@color/white" | ||
| RectangleButtonWithText:roundButtonTextSize="6sp" | ||
| android:drawableRight="@drawable/connect_right_arrow" | ||
| /> | ||
| <!--<Button | ||
| android:id="@+id/login_button" | ||
| android:layout_width="fill_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="bottom" | ||
| android:nextFocusUp="@+id/edit_password" | ||
| android:paddingTop="@dimen/content_start" | ||
| android:paddingBottom="@dimen/content_start" />--> | ||
|
|
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
Update MaterialButton implementation for consistency.
The button implementation needs the following improvements:
- Replace deprecated
drawableRightwith Material's icon support - Add consistent styling attributes
<com.google.android.material.button.MaterialButton
android:id="@+id/login_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="end"
android:layout_marginVertical="24dp"
android:text="Login with Connect ID"
- android:drawableRight="@drawable/connect_right_arrow"
+ app:icon="@drawable/connect_right_arrow"
+ app:iconGravity="end"
+ app:iconTint="?attr/colorOnPrimary"
+ android:textAllCaps="false"
+ app:cornerRadius="8dp"
+ app:elevation="2dp"/>📝 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.
| <com.google.android.material.button.MaterialButton | |
| android:id="@+id/login_button" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="end" | |
| android:layout_marginVertical="24dp" | |
| android:text="Login with Connect ID" | |
| RectangleButtonWithText:roundButtonBackgroundColor="@color/connect_blue_color" | |
| RectangleButtonWithText:roundButtonIcon="@drawable/connect_right_arrow" | |
| RectangleButtonWithText:roundButtonTextColor="@color/white" | |
| RectangleButtonWithText:roundButtonTextSize="6sp" | |
| android:drawableRight="@drawable/connect_right_arrow" | |
| /> | |
| <!--<Button | |
| android:id="@+id/login_button" | |
| android:layout_width="fill_parent" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="bottom" | |
| android:nextFocusUp="@+id/edit_password" | |
| android:paddingTop="@dimen/content_start" | |
| android:paddingBottom="@dimen/content_start" />--> | |
| <com.google.android.material.button.MaterialButton | |
| android:id="@+id/login_button" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="end" | |
| android:layout_marginVertical="24dp" | |
| android:text="Login with Connect ID" | |
| app:icon="@drawable/connect_right_arrow" | |
| app:iconGravity="end" | |
| app:iconTint="?attr/colorOnPrimary" | |
| android:textAllCaps="false" | |
| app:cornerRadius="8dp" | |
| app:elevation="2dp"/> |
|
@damagatchi retest this please |
Rounded button as per the figma design of connect
Product Description
PR Checklist
Automated test coverage
Safety story