-
Notifications
You must be signed in to change notification settings - Fork 26
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
refactor: introduce BasicTextField2 [WPB-8779] [WPB-8727] #2991
refactor: introduce BasicTextField2 [WPB-8779] [WPB-8727] #2991
Conversation
Ups 🫰🟨This PR is too big. Please try to break it up into smaller PRs. |
Build 4692 failed. |
Build 4693 failed. |
APKs built during tests are available here. Scroll down to Artifacts! |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 4716 succeeded. The build produced the following APK's: |
app/src/main/kotlin/com/wire/android/ui/common/textfield/CodeTextField.kt
Show resolved
Hide resolved
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.
Amazing work!
Just have a small nitpick
app/src/main/kotlin/com/wire/android/ui/common/textfield/CodeTextField.kt
Show resolved
Hide resolved
…field2_with_temp_wrapper
APKs built during tests are available here. Scroll down to Artifacts! |
Build 4726 succeeded. The build produced the following APK's: |
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.
Looks awesome.
Just a small question and a nitpick 🙃
app/src/main/kotlin/com/wire/android/ui/common/textfield/StateSyncingModifier.kt
Show resolved
Hide resolved
app/src/main/kotlin/com/wire/android/ui/common/textfield/StateSyncingModifier.kt
Outdated
Show resolved
Hide resolved
…SyncingModifier.kt Co-authored-by: Vitor Hugo Schwaab <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2991 +/- ##
===========================================
- Coverage 44.19% 44.15% -0.05%
===========================================
Files 447 447
Lines 14548 14548
Branches 2498 2498
===========================================
- Hits 6429 6423 -6
- Misses 7417 7424 +7
+ Partials 702 701 -1
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
APKs built during tests are available here. Scroll down to Artifacts! |
Build 4739 succeeded. The build produced the following APK's: |
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
There are some issues with current text fields, numerous crashes or weird behaviours. Google noticed that some time ago and started working on v2.
This PR is introducing
BasicTextField2
into our project, replacing all our current text fields with the new version, at this moment temporarily still using our logic with passingTextFieldValue
andonValueChanged
callbacks. Ultimately, all text fields should be migrated to useTextFieldState
.In this PR:
1.7.0-alpha05
, from whichBasicTextField2
has been marked as stable; in versions prior to chosen one, including current newest release version -1.6.7
,BasicTextField2
has some bad issues with the keyboard (https://issuetracker.google.com/issues/339171226) and interaction source (https://issuetracker.google.com/issues/327665606).1.7.0
version,rememberRipple()
needs to be updated toripple()
andAnchoredDraggableState
also requiresdecayAnimationSpec
TextFieldValue
andonValueChanged
and second one already withTextFieldState
- it applies toWireTextField
,WirePasswordTextField
andCodeTextField
AutoFillTextField
is removed and auto fill option is added as a parameter to theWireTextField
as it doesn't make sense to keep two almost the same classesWireTextField
andWirePasswordTextField
there isWireTextFieldLayout
reused, and forCodeTextField
there is aCodeTextFieldLayout
StateSyncingModifier
which is based on compose BasicTextFieldWithValueOnValueChangeSample which allows to synchronise betweenTextFieldState
andTextFieldValue
withonValueChanged
callbackNotes:
BasicTextField2
has been marked as stable and renamed to regularBasicTextField
, so it's quite confusing, but the difference is that new one takesTextFieldState
instead ofTextFieldValue
orString
Testing
How to Test
All text fields has been updated so ideally all of them should be tested and checked if they behave the same.
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.