App
App copied to clipboard
Expensify
[$500] IOU - Destinations used for creating request in offline remains grayed out in online mode
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.38-0 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): [email protected] Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
Action Performed:
- Navigate to staging.new.expensify.com as Employee
- Disable the internet connection
- Go to Workspace chat> +> Money request> Distance> Create request
- Go back online
- Go to Workspace chat> +> Money request> Distance
- Take a note of recent destination results
Expected Result:
All recent destination should not be grayed out
Actual Result:
Destinations used for creating request in offline remains grayed out in online mode
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/376338fc-9449-4f55-96a4-10775e8edc7f
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01d576261e97da21a3
- Upwork Job ID: 1755338770224349184
- Last Price Increase: 2024-02-07
Job added to Upwork: https://www.upwork.com/jobs/~01d576261e97da21a3
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)
Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Proposal
Please re-state the problem that we are trying to solve in this issue.
IOU - Destinations used for creating request in offline remains grayed out in online mode
What is the root cause of that problem?
The main problem with issue is that destinations used for creating request in offline have only subtitle Which is always gray
https://github.com/Expensify/App/blob/c5f84b6a5c744e020fbb467f085931ce81534c37/src/components/AddressSearch/index.tsx#L349-L352
What changes do you think we should make in order to solve the problem?
I think the best way to fix this issue In case when we don't have a title we will use subtitle and vice versa
{(title || subtitle) && <Text style={[styles.googleSearchText]}>{title || subtitle}</Text>}
<Text style={[styles.textLabelSupporting]}>{subtitle || title}</Text>
Or if we expect for destinations used for creating request in offline to have only one line So in this case we can use only title component in case when we have only one title or subtitle
if (title && subtitle) {
return (
<View>
<Text style={[styles.googleSearchText]}>{title}</Text>
<Text style={[styles.textLabelSupporting]}>{subtitle}</Text>
</View>
);
}
return (
<View>
<Text style={[styles.googleSearchText]}>{title || subtitle}</Text>
</View>
);
But this is more of a question for the designers
What alternative solutions did you explore? (Optional)
As alternative we can pass description for name when name is undefined to show always 2 strings when we select location (In offline mode name is undefined, and text from input is saved like description )
https://github.com/Expensify/App/blob/c5f84b6a5c744e020fbb467f085931ce81534c37/src/components/AddressSearch/index.tsx#L93-L98
Proposal
Please re-state the problem that we are trying to solve in this issue.
When a Distance request is created while offline, the entered locations are saved and later displayed in the recent locations but they show "grayed out".
What is the root cause of that problem?
When the user searches for a waypoint in offline mode, the app will just take the input as is as it can't ask Google for a translation for that waypoint. Because of that the waypoint is saved with an address only, no name.
https://github.com/Expensify/App/blob/3d79c9c54ea1cd616adf4af3e898d55302d614a4/src/pages/iou/request/step/IOURequestStepWaypoint.js#L150-L158
Later on when the recent waypoint is displayed, we do the following:
https://github.com/Expensify/App/blob/3d79c9c54ea1cd616adf4af3e898d55302d614a4/src/components/AddressSearch/index.tsx#L346-L355
This will render the bolded name first and then the description. Description is a copy of the address value as can be seen here:
https://github.com/Expensify/App/blob/3d79c9c54ea1cd616adf4af3e898d55302d614a4/src/pages/iou/request/step/IOURequestStepWaypoint.js#L288-L297
Because some recent waypoints have been saved in offline mode, they do not have a name field so it will only show the description without the bolded name, giving the impression of being greyed out. This is not really grayed out in the sense that it's offline/not available, it's just that it doesn't display the bolded title because it doesn't have one.
What changes do you think we should make in order to solve the problem?
Assuming we want to display it bolded as the others, we update the following code:
https://github.com/Expensify/App/blob/3d79c9c54ea1cd616adf4af3e898d55302d614a4/src/components/AddressSearch/index.tsx#L349-L354
To be like:
return (
<View>
{title && <Text style={[styles.googleSearchText]}>{title}</Text>}
<Text style={[title ? styles.textLabelSupporting : styles.googleSearchText]}>{subtitle}</Text>
</View>
);
This will display the description in bold if there is no title.
What alternative solutions did you explore? (Optional)
Another option is to simply do not list any "incomplete" (those created offline) waypoints on the recent waypoints list. As noted here, sometimes an waypoint created offline turns out to be invalid, and we likely don't way to show these in the list of recent waypoints.
The way we identify these is by looking at lat and lng as these will be null for waypoints created offline (and they will remain null once back online if the waypoint is invalid). Here's where we would filter the waypoints:
src/pages/iou/request/step/IOURequestStepWaypoint.js:
waypoints.filter((w) => w.lat && w.lng) added to the _.map call.
recentWaypoints: {
key: ONYXKEYS.NVP_RECENT_WAYPOINTS,
// Only grab the most recent 5 waypoints because that's all that is shown in the UI. This also puts them into the format of data
// that the google autocomplete component expects for it's "predefined places" feature.
selector: (waypoints) =>
_.map(waypoints ? waypoints.filter((w) => w.lat && w.lng).slice(0, 5) : [], (waypoint) => ({
name: waypoint.name,
description: waypoint.address,
geometry: {
location: {
lat: waypoint.lat,
lng: waypoint.lng,
},
},
})),
},
This would solve the issue of showing invalid waypoints in the recent list, but as noted here, some valid waypoints, created online, could still not have name, which would leave us with the original issue we're dealing with here ("grayed out" suggestion).
The best solution here seems to be 1) filter invalid waypoints; 2) bold description when name is not available. This solves both issues. Or, if the team prefers, we hide anything without a name.
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
I don't think this is a bug. I think that current behaviour gives the user a hint that the selected address was saved in offline mode.
@puneetlath What's your opinion on this? should we make any adjustments?
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Hmmmm, interesting.
Because some recent waypoints have been saved in offline mode, they do not have a name field so it will only show the description without the bolded name, giving the impression of being greyed out. This is not really grayed out in the sense that it's offline/not available, it's just that it doesn't display the bolded title because it doesn't have one.
Is the only way to not have a "name" to add it via offline mode? Or are there other ways in which you can end up with a recent destination that doesn't have a name?
@puneetlath, @fedirjh Eep! 4 days overdue now. Issues have feelings too...
Hmmmm, interesting.
Because some recent waypoints have been saved in offline mode, they do not have a name field so it will only show the description without the bolded name, giving the impression of being greyed out. This is not really grayed out in the sense that it's offline/not available, it's just that it doesn't display the bolded title because it doesn't have one.
Is the only way to not have a "name" to add it via offline mode? Or are there other ways in which you can end up with a recent destination that doesn't have a name?
Technically it could be empty even in online mode, if the waypoint returned by Google doesn't have a name.
https://github.com/Expensify/App/blob/9c4c4ac9a457df2877885a15a5d7243eae00fb50/src/pages/iou/request/step/IOURequestStepWaypoint.js#L176-L189
Line 181, values.name could potentially be null.
Ok got it. In that case, I think displaying the description in bold when there is no name makes sense. What do you think @fedirjh?
Though actually, will those destinations created in Offline Mode work? I just tried with the locations "Atlanta" and "New York" and the request ultimately failed to be created. So maybe a better solution is to not add the destination to the recents list unless it is one that actually comes from Google.
Proposal
Updated my proposal to include an alternate solution to deal with the issue @puneetlath mentioned above (invalid waypoints created offline). The final solution would be a hybrid where we hide invalid waypoints and also bold description when no name is available. It also covers hiding it altogether if name is not present in case that's the route we want to go.
@puneetlath @fedirjh this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Though actually, will those destinations created in Offline Mode work? I just tried with the locations "Atlanta" and "New York" and the request ultimately failed to be created. So maybe a better solution is to not add the destination to the recents list unless it is one that actually comes from Google.
Yes if you use an invalid address offline, then it won't work. There isn't a good way to validate addresses while offline, so we optimistically assume that they are valid. I think we should continue to optimistically add to the recent waypoints so that users can make multiple offline requests easily, assuming they actually use valid addresses. It's tricky and I'm still open to other ideas about how it should work.
Displaying the description in bold when there is no name sounds like a good solution here.
@puneetlath, @fedirjh Eep! 4 days overdue now. Issues have feelings too...
Ok got it. In that case, I think displaying the description in bold when there is no name makes sense.
This looks to me good as well.
Thanks @ZhenjaHorbach and @barros001 for the proposals. Both proposals look correct; However, I preferred @barros001's solution of changing the style title ? styles.textLabelSupporting : styles.googleSearchText as it looks simpler.
@puneetlath @fedirjh this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
📣 @fedirjh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
📣 @barros001 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Keep in mind: Code of Conduct | Contributing 📖
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.4.47-10 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/37462
If no regressions arise, payment will be issued on 2024-03-13. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @barros001 requires payment automatic offer (Contributor)
- @fedirjh requires payment automatic offer (Reviewer)