-
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 2022-03-22] Searching non existing currency results in a blank page with active Confirm button - reported by @Tushu17 #7966
Comments
Triggered auto assignment to @sakluger ( |
Reproduced this issue, and clarified the action steps in the original GH issue. Ready for engineering. 👌 |
Triggered auto assignment to @timszot ( |
Triggered auto assignment to @NicMendonca ( |
Looks good to me, sending to external for better handling of this case in the app. |
Upwork job posting: https://www.upwork.com/jobs/~0117a680819923466f |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel ( |
Triggered auto assignment to @luacmartins ( |
Issue looks good. I'd just change the expected behaviour to:
|
I suggest to update IOUCurrencySelection.js so that sectionlist has listemptycomponent set to show No results found when data is empty, and a state to hold sectionlist's data |
@laravue1118 You should be new here. Check out our contributing guidelines first. And go through these closed issues here for reference. And make a proper proposal with all the changes you would make to solve this issue. |
Proposal {this.state.currencyData.length === 0 ? (
<View style={[styles.ph5, styles.pb5]}>
<Text style={[styles.textLabel, styles.colorMuted]}>
{this.props.translate('common.noResultsFound')}
</Text>
</View>
)
: null } we can add the code given above after line 181. By this we can show the no results found message below the search input App/src/pages/iou/IOUCurrencySelection.js Lines 176 to 181 in 3d2ec23
And to remove the getSections() {
const sections = [];
if (this.state.currencyData.length > 0) {
sections.push({
title: this.props.translate('iOUCurrencySelection.allCurrencies'),
data: this.state.currencyData,
shouldShow: true,
indexOffset: 0,
});
}
return sections;
} |
ProposalI will update getSections function.
` I will update rendering of sectionlist. `
` And in the confirmCurrencySelection, I will check if the data is empty and show error. `
` |
I will check on this by Wednesday. |
Hmm I actually think it would be ok to keep the |
@luacmartins, This is how it will look if we keep the |
I like that better, but I'll ask for @Expensify/design input here. |
I had proposed a solution here in case it got missed. I have a better approach for both the options - {this.state.currencyData.length === 0 && (
<Text style={[styles.ph5, styles.textLabel, styles.colorMuted]}>
{this.props.translate('common.noResultsFound')}
</Text>
)} If we wanna keep the App/src/pages/iou/IOUCurrencySelection.js Lines 205 to 212 in 3d2ec23
If we don't want the renderSectionHeader={({section: {title}}) => (
<View>
{this.state.currencyData.length === 0 ? (
<Text style={[styles.ph5, styles.textLabel, styles.colorMuted]}>
{this.props.translate('common.noResultsFound')}
</Text>
) : (
<Text style={[styles.p5, styles.textMicroBold, styles.colorHeading]}>
{title}
</Text>
)}
</View>
)} cc: @luacmartins |
When there are no results based on your search, I think I prefer it without the "ALL CURRENCIES" header. |
Thanks @shawnborton! @thesahindia I like your updated approach, but can you elaborate on how you'll display the error when the user taps |
@luacmartins, I think we don't need an error here as one option always stays selected, so when the user taps confirm the currency that was already selected will be used. |
Hmm that makes sense. Alright then, let's move forward with your updated approach and removing the |
📣 @thesahindia You have been assigned to this job by @luacmartins! |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.42-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 2022-03-22. 🎊 |
@thesahindia Can you please apply to the job in Upwork so I can issue payment? - https://www.upwork.com/jobs/~0117a680819923466f |
Done ✅ |
@Santhosh-Sellavel @Tushu17 @thesahindia paid - thank you! |
@NicMendonca When you have time please end the contract on the Upwork, thanks! |
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:
It should show something like "no results found" and Confirm button should be disabled.
Actual Result:
It shows a blank results page with an active continue button.
Workaround:
unknown
Platform:
Where is this issue occurring?
Version Number: 1.1.41-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
screen-rec.2.mp4
Expensify/Expensify Issue URL:
Issue reported by: @Tushu17
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1644878029333329
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: