-
Notifications
You must be signed in to change notification settings - Fork 120
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
[Multiple Choice Task] Fix UI bugs in multiple choice selectors #2899
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2899 +/- ##
============================================
+ Coverage 62.58% 62.66% +0.07%
Complexity 1203 1203
============================================
Files 267 267
Lines 6356 6358 +2
Branches 877 873 -4
============================================
+ Hits 3978 3984 +6
Misses 1831 1831
+ Partials 547 543 -4
|
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.
Please paste a screenshot into the PR desc when you can. Thanks!
android:inputType="text" | ||
android:text="@{item.otherText}" | ||
android:textColor="@color/md_theme_onSurface" | ||
android:textSize="16sp" |
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 believe you'd want to extend Widget.MaterialComponents.TextInputEditText.FilledBox
here.
android:onClickListener="@{() -> viewModel.toggleItem(item)}" | ||
android:padding="16dp" | ||
android:text="@{ item.isOtherOption ? @string/other : item.option.label }" | ||
android:textColor="@color/md_theme_onSurface" | ||
android:textSize="16sp" |
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 you define this in styles.xml
instead using the Material theme names from Figma here and throughout? I think you need to extend Widget.Material3.CompoundButton.CheckBox
.
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.
How about taking this opportunity to migrate it to jetpack compose?
Marking this as draft. Working towards migrating the task's UI to compose. |
Fixes #2893
Updates UX as per figma mocks
@gino-m PTAL?