-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Pm rounded button #2897
Pm rounded button #2897
Changes from 7 commits
e850fbb
06e6611
2b46881
eeab8d2
573ca4d
6cc54e7
b7156c8
10eadc6
3ae3643
ad7c456
a3af6b6
8e070fd
9f15541
99bc8f8
ccf96c2
1754bda
42a347b
e7cd5e8
29eb7f7
affabbf
1d4a933
cfe8594
ed3c688
7ea3cf6
b79797c
2c5eefe
2182e23
063e95c
53befeb
5f33c64
732c706
1def181
322e1aa
4ac0f10
3dcb2f1
1e5f9d8
5f22c1a
6c9ba8c
3ea45c8
9365782
c06692c
67d2919
44c763e
edbc979
13ae017
180db27
dde40f5
2e275f5
9e87ddb
4ef0149
eb6ce41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,32 +44,32 @@ | |
android:layout_width="match_parent" | ||
android:orientation="horizontal"> | ||
|
||
<Button | ||
style="@style/MapButtonGreen" | ||
pm-dimagi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<com.google.android.material.button.MaterialButton | ||
android:backgroundTint="@color/green_500" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old styled defined a selector that changes based on pressed button state, any reason we are no longer doing that ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see anything in response to my comment above, as so curious why was this resolved ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see anything in response to my comment above, as so curious why was this resolved ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is updated in the last commit i have created the new style for this which is of same behaviour as the previous one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you please link me to the style you are referring to ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shubham by mistake i have made that correction in title bar color change pr . You can check the code over here |
||
android:id="@+id/start_tracking_button" | ||
android:layout_height="wrap_content" | ||
android:layout_weight="1" | ||
android:visibility="gone" | ||
android:layout_width="0dp" /> | ||
|
||
<Button | ||
style="@style/MapButtonRed" | ||
<com.google.android.material.button.MaterialButton | ||
android:id="@+id/stop_tracking_button" | ||
android:backgroundTint="@color/red_500" | ||
android:layout_height="wrap_content" | ||
android:layout_weight="1" | ||
android:layout_width="0dp" | ||
android:visibility="gone" /> | ||
|
||
<Button | ||
style="@style/MapButtonOrange" | ||
<com.google.android.material.button.MaterialButton | ||
android:backgroundTint="@color/orange_500" | ||
android:id="@+id/redo_tracking_button" | ||
android:layout_height="wrap_content" | ||
android:layout_weight="1" | ||
android:layout_width="0dp" | ||
android:visibility="gone" /> | ||
|
||
<Button | ||
style="@style/MapButtonGreen" | ||
<com.google.android.material.button.MaterialButton | ||
android:backgroundTint="@color/green_500" | ||
android:id="@+id/ok_tracking_button" | ||
android:layout_height="wrap_content" | ||
android:layout_weight="1" | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -56,21 +56,19 @@ | |||||||||||||||||||||||||||||||||||
android:maxLines="3" | ||||||||||||||||||||||||||||||||||||
android:minLines="3" /> | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
<Button | ||||||||||||||||||||||||||||||||||||
<com.google.android.material.button.MaterialButton | ||||||||||||||||||||||||||||||||||||
android:id="@+id/confirm_location_button" | ||||||||||||||||||||||||||||||||||||
android:layout_width="fill_parent" | ||||||||||||||||||||||||||||||||||||
android:layout_height="wrap_content" | ||||||||||||||||||||||||||||||||||||
android:layout_gravity="bottom" | ||||||||||||||||||||||||||||||||||||
android:enabled="false" | ||||||||||||||||||||||||||||||||||||
style="@style/Commcare.Button.Primary" | ||||||||||||||||||||||||||||||||||||
android:text="@string/confirm_location" /> | ||||||||||||||||||||||||||||||||||||
Comment on lines
+59
to
65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add visual feedback for disabled state The confirm location button is disabled by default, but there's no clear visual indication of this state. Add style and state list animator: <com.google.android.material.button.MaterialButton
android:id="@+id/confirm_location_button"
android:layout_width="fill_parent"
android:layout_height="wrap_content"
android:layout_gravity="bottom"
android:enabled="false"
+ style="@style/Widget.MaterialComponents.Button"
+ android:stateListAnimator="@null"
android:text="@string/confirm_location" /> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
<Button | ||||||||||||||||||||||||||||||||||||
<com.google.android.material.button.MaterialButton | ||||||||||||||||||||||||||||||||||||
android:id="@+id/cancel_button" | ||||||||||||||||||||||||||||||||||||
android:layout_width="fill_parent" | ||||||||||||||||||||||||||||||||||||
android:layout_height="wrap_content" | ||||||||||||||||||||||||||||||||||||
android:layout_gravity="bottom" | ||||||||||||||||||||||||||||||||||||
style="@style/Commcare.Button.Primary" | ||||||||||||||||||||||||||||||||||||
android:text="@string/cancel" /> | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
</LinearLayout> | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,13 +1,14 @@ | ||||||
<?xml version="1.0" encoding="utf-8"?> | ||||||
<Button xmlns:android="http://schemas.android.com/apk/res/android" | ||||||
<com.google.android.material.button.MaterialButton xmlns:android="http://schemas.android.com/apk/res/android" | ||||||
shubham1g5 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
xmlns:tools="http://schemas.android.com/tools" | ||||||
xmlns:app="http://schemas.android.com/apk/res-auto" | ||||||
style="@style/CustomButtonStyleOutline" | ||||||
app:strokeColor="@color/primary_button_background" | ||||||
android:id="@+id/blue_outlined_button" | ||||||
android:layout_width="wrap_content" | ||||||
android:layout_height="wrap_content" | ||||||
android:layout_gravity="right" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Update deprecated layout_gravity value Replace - android:layout_gravity="right"
+ android:layout_gravity="end" 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @avazirna this would be nice to correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
android:textColor="@color/blue" | ||||||
android:visibility="gone" | ||||||
style="@style/CommCare.Button.Default" | ||||||
android:background="@drawable/blue_outlined_view" | ||||||
android:padding="10dp" | ||||||
tools:viewBindingIgnore="true"/> | ||||||
tools:viewBindingIgnore="true" /> |
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
Add missing attributes for MaterialButton
The MaterialButton implementation needs the following improvements:
Apply this diff to enhance the button implementation: