-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
When blurring the emoji picker search field, it wobbles if it is not empty #26436
Comments
Triggered auto assignment to @laurenreidexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Proposal from @joh42 ProposalPlease re-state the problem that we are trying to solve in this issue.When blurring the emoji picker search field, it wobbles if it is not empty. What is the root cause of that problem?The root cause of the issue is that the BaseTextInput component checks the length of the App/src/components/TextInput/BaseTextInput.js Lines 112 to 117 in 44eb88a
But the EmojiPickerMenu does not pass any value prop: App/src/components/EmojiPicker/EmojiPickerMenu/index.js Lines 536 to 549 in 44eb88a
This means that even if the text input is not empty, label deactivation will begin, leading to the buggy animation. Previously, the value of the input was stored inside of the BaseTextInput component, but there was a conscious decision made to remove that in this PR. Since then the What changes do you think we should make in order to solve the problem?There is already a ref, called App/src/components/TextInput/BaseTextInput.js Lines 184 to 186 in 44eb88a
I suggest we check that ref as well in the App/src/components/TextInput/BaseTextInput.js Lines 112 to 117 in 44eb88a
Which would look something like this:
If the input has a value, we do not want the deactivation animation of the label to start. We could likely remove the check of the length of the value prop entirely in that conditional, since checking This approach makes the most sense IMO. I think hinging the animation logic on the length of a prop that isn't required is suboptimal when the BaseTextInput component already stores whether or not the text input is empty. What alternative solutions did you explore? (Optional)Alternative solution 1Alternatively we could pass the
App/src/components/EmojiPicker/EmojiPickerMenu/index.js Lines 419 to 420 in fc9378e
As such we would have to start storing the search term in both the regular EmojiPickerMenu and the native EmojiPickerMenu just so that the BaseTextInput label can animate correctly. If we choose this approach, I think we should consider making the Alternative solution 2Another alternative would be to start storing the current input value in the BaseTextInput component again. I do not really think this is worth considering, given that a decision has been made to not store the current input value in the BaseTextInput component, but it is an option. It is also not really required, since we are only interested in whether or not the input is empty, not the actual value of the input. |
Should be fixed in #26399 |
Closing for #26399 |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
The emoji search field should behave like other input fields - it should not wobbl
Actual Result:
The emoji search field wobbles
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.3.60-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2023-08-22.At.20.13.38.mp4
Recording.1413.mp4
Expensify/Expensify Issue URL:
Issue reported by: @joh42
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692728174404989
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: