-
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-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$1000] Web - When input exceeds 500k characters, the web freezes. #25804
Comments
Triggered auto assignment to @johncschuster ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.When input exceeds 500k characters, the web freezes. What is the root cause of that problem?There is no What changes do you think we should make in order to solve the problem?To achieve the desired design, we need
updateSearchValue(value){
if (value.length > this.props.maxLength) {
this.setState({
errorMessage: this.props.translate("common.error.characterLimitExceedCounter", {length: value.length, limit: this.props.maxLength}),
searchValue: value,
});
return;
}
this.setState({
errorMessage: "",
searchValue: value,
});
this.props.onChangeText(value);
}
<TextInput
value={this.state.searchValue}
onChangeText={this.updateSearchValue}
errorText={this.state.errorMessage}
maxLength={this.props.maxLength + CONST.ERROR_EXCEED_RANGE}
headerMessage={this.state.errorMessage ? "" : this.props.headerMessage} Screen.Recording.2023-10-03.at.12.29.53.AM.movWhat alternative solutions did you explore? (Optional)N/A |
Job added to Upwork: https://www.upwork.com/jobs/~01ffb0be0940793404 |
Current assignee @johncschuster is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts ( |
Not overdue. The issue has just been triaged to |
ProposalPlease re-state the problem that we are trying to solve in this issue.Inputting too many characters in the input for contact search causes the page to crash and require a refresh of the page. What is the root cause of that problem?RegExp throws this uncaught error: Further debugging shows that the error is thrown from this line: App/src/libs/OptionsListUtils.js Line 561 in cdec13d
What changes do you think we should make in order to solve the problem?Add a string length check in Line 107 in cdec13d
This will inform the user of the limitations and prevent this from crashing the page. This will also take advantage of preexisting and future translations. What alternative solutions did you explore? (Optional)(Blank) |
📣 @meldevigolsen! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@johncschuster, @allroundexperts Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@jeet-dhandha I don't think that your proposal is tackling the correct problem here. |
@shawnborton I would like to confirm what the expected behaviour should be design wise. Should we display an error if the input length exceeds the allowed length or just not let the user enter anything more in the input without showing them any error? On a side note, this error exists in all of the OptionSelector fields. We should aim to fix this in all such fields. |
I know my proposal isn't directly solving the issue. As main issue is the freezing of web which is mostly due to pre-processing of emojis. |
That's not right. The issue is for option selector inputs instead of the composer. |
I don't feel strongly here. This feels like such an edge case that a user would actually type 500 characters into a text input that I am not too worried about needing an explicit error message, I think even just preventing any more characters would be fine. Curious what @dannymcclain @JmillsExpensify and @trjExpensify think though. In terms of the option selector, can you show me what you mean by that? |
500k characters? As in, 500,000? Haha, wild. We have this issue to reduce the character limits to something more reasonable across the board. Interestingly, @joekaufmanexpensify and I stumbled upon this same topic last week in an issue and I believe he was bringing it out to Slack. I think we should follow the form pattern here really to show an in-line error on blur. Additionally, Joe had the idea to then show a character limit counter if it has been exceeded, so you know how many you need to delete to get back under the allowed limit. I think that's a good approach personally. Different fields have different lengths, so an error message to highlight what it is, is more helpful than preventing further typing or auto-truncating if you paste more characters than allowed. |
Contributor details |
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@flodnv, @johncschuster, @allroundexperts, @ahmedGaber93 Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Need a payment summary for this issue. |
Payment Summary@allroundexperts does not require payment (Eligible for Manual Requests) - $1000 - paid via NewDot |
$1,000 payment approved for @allroundexperts based on summary above. |
@allroundexperts for the regression test, I think this is a typo:
That's just meant to be "exceeds 500 characters", right? |
@johncschuster Yes 🙈 |
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
Looks like the job closed on Upwork. I'll create a payment issue and add @ahmedGaber93 and @LeThiThuThuy to that. |
Ah, it looks like @LeThiThuThuy has been updated to @thuyle04. I will add them to the payment issue. |
@ahmedGaber93, please comment on the above GH so I can assign it to you for payment. Thank you! |
@johncschuster Ok, commented. |
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@ahmedGaber93 I don't see your comment on #30399. Can you please try again? Once you've commented, I can assign it and you'll be invited to the job. |
Ok, commented recheck @johncschuster |
@flodnv, @johncschuster, @allroundexperts, @ahmedGaber93 Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@flodnv, @johncschuster, @allroundexperts, @ahmedGaber93 Eep! 4 days overdue now. Issues have feelings too... |
Payment is being issued here. I'm going to close this issue. |
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 website does not crash when entering long text.
Actual Result:
The website is frozen or crash
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.3.56-21
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
Crash.1.mp4
Crash.2.mp4
Recording.1308.mp4
Expensify/Expensify Issue URL:
Issue reported by: @LeThiThuThuy
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691988614374279
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: