-
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
[HOLD for payment 2023-10-10] [$500] [Distance] - The system does not display "No result found" when entering an invalid address. #27165
Comments
Triggered auto assignment to @trjExpensify ( |
Job added to Upwork: https://www.upwork.com/jobs/~01f72156da7ba00e25 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to @tjferriss ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @burczu ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.[Distance] - The system does not display "No result found" when entering an invalid address What is the root cause of that problem?In waypointEditor, we pass predefinedPlaces props so we always have dataList It caused ListEmptyComponent (In this case: "No result found" Text) don't display What changes do you think we should make in order to solve the problem?
because we defined default resultTypes is address
If the textValue is not empty and there are some search result we will don't display predefinedPlaces If the textValue is not empty and there are no search result we will display no result found We also can easily move step 2,3 to our react-native-google-places-autocomplete fork. Result |
ProposalPlease re-state the problem that we are trying to solve in this issue.When searching for an address in the Distance Request flow, the autocomplete input does not display "No results found" for an address with no results. What is the root cause of that problem?This is due to the fact that What changes do you think we should make in order to solve the problem?Since Expensify maintains a fork of this module, we can change this behaviour if absolutely necessary. Essentially, we will force the no results empty component to show only after a request has been made, and no results were found.
let res = [];
+if (results.length === 0 && showNoResults) {
+ return [];
+}
_results = results;
-setDataSource(buildRowsFromResults(results));
+setDataSource(buildRowsFromResults(results, true)); What alternative solutions did you explore? (Optional)None. |
ProposalPlease re-state the problem that we are trying to solve in this issue.A user enters the same address in two places: Given the same prompt, field (1) displays "No results found", but field (2) displays a list of selectable addresses. What is the root cause of that problem?Field (1) fires an API request proxied to Google Maps API with filtering by place's type. Comparison of API requests in two fields, given the same prompt There are two root causes of the problem:
What changes do you think we should make in order to solve the problem?
What alternative solutions did you explore? (Optional)N/A |
📣 @needpower! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
I've checked the proposals and done my own investigation and it seems like to obtain exactly the same behaviour in Distance Address field as it is done in Personal Details Address field we need to mix two proposals into one. If we only follow only the @DylanDylann's proposal we will get results like below: This is because filtering by address is disabled for Distance page, as @needpower pointed out in his proposal. If we additionally remove That being said, I think we should hire one of them to implement merged solution, and split the payment between them. 🎀 👀 🎀 C+ reviewed P.S. After additional check we should probably not remove |
Triggered auto assignment to @youssef-lr, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@neil-marcellini @hayata-suenaga @tgolen can I get your input on this one? My initial take was that it wasn't a bug. If you have recent addresses when you're on the waypoint picker, you should see those instead of "no address found", though maybe there is some merit in being more explicit with "no address found" when your search input doesn't match any of your recent addresses for better feedback than this: |
@trjExpensify yeah - I agree that maybe keeping the same behaviour in both places may not be the best option in the first place... |
I agree that I do not feel strongly that these need to work exactly the same. The use case is different. One of them has the recent waypoints, and the other one does not. Therefore, there is different behavior for invalid address search. I think IF we wanted them to be consistent, then it would be better to maintain a separate "recent address" collection for the profile search. |
@tgolen I think from a UX perspective, users search and select distance addresses much more often than changing a home address. It makes sense to remember previous choices to facilitate the distance search for the case when a user what to use the same route in a money request. |
Yeah, I totally agree we don't need recent addresses in the home address search. My only question comes down to whether "no address found" should be displayed if you enter an invalid address into the search bar for feedback that what you're typing is yielding no results. I do find this a bit strange: qqYl9sP9nJ.mp4 |
Agreed. I find that weird. We should be giving feedback for no results. |
giving an explicit feedback that a result was not found might be a good idea |
Yeah I think it just default to the recent result as it does not have the backend results yet. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.76-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-10-10. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
👋 Can we get this checklist filled out to proceed with payments please? |
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
Again here @burczu, there's no justification noted for this. I'd love to be able to read minds, but unfortunately I can't, so it would be good for you to communicate your reasoning. |
@burczu, @youssef-lr, @trjExpensify, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@burczu, @youssef-lr, @trjExpensify, @DylanDylann Huh... This is 4 days overdue. Who can take care of this? |
I think in this case it's just an UX fix, it was still possible to use distance selectors and the whole request money flow based on distance so the issue didn't break any flow in the App. |
Got it. I guess I tend to agree that it's a fairly minor improvement to add to the regression test run standalone. I just took a cursory look through the distance requests ones and we're covered for things like auto-complete, placenames and no address found if you try to save, so I think that's sufficient without an explicit one for returning no results. |
Proceeding on with payments then:
No urgency bonus to apply as per here. Once confirmed, I'll proceed to settle up. |
@burczu, @youssef-lr, @trjExpensify, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Settled up with @DylanDylann. @usmantariq96, I've sent you an offer. |
Settled up with @usmantariq96, closing! |
sorry @trjExpensify but it seems I wasn't paid out on the job yet. Could you help double check, thank you! |
Cool yeah, I reached out to yah on Slack as it looked like it failed but you never know with Upwork! I've now settled up with you. |
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:
Upon entering an invalid address, the system should display "No result found."
Actual Result:
The system does not display "No result found" when entering an invalid address.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.67.2
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
Bug.3.mp4
Recording.4388.mp4
Expensify/Expensify Issue URL:
Issue reported by: @usmantariq96
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694077781824689
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: