-
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
[$500] Chat - There's a delay for showing the "over 10 000" message again #33612
Comments
Job added to Upwork: https://www.upwork.com/jobs/~0104997dec9d66e892 |
Triggered auto assignment to @JmillsExpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Error message What is the root cause of that problem?The method used to check the max comment length
What changes do you think we should make in order to solve the problem?This seems to be intentional to prevent multiple invocations within a short duration. To fix the initial delay, set debounce method to invoke immediately on first execution by setting const validateCommentMaxLength = useMemo(() => _.debounce(handleValueChange, 1500, { leading: true }), [handleValueChange]); What alternative solutions did you explore? (Optional)Instead of completely removing it, we could reduce the timeout to 500ms, which will ensure that the error will be shown much faster. Also the validation will get executed only a maximum of 2 times with in a second. // update timeout to 500ms
const validateCommentMaxLength = useMemo(() => _.debounce(handleValueChange, 500), [handleValueChange]); |
Contributor details Proposal Please re-state the problem that we are trying to solve in this issue. Error message The maximum comment length is 10,000 characters is only shown after a delay. What is the root cause of that problem? The method that is used to validate the content length validateCommentMaxLength is fired off every time the view loads, which causes the debounce to fire and the message to show up 1.5 seconds later What changes do you think we should make in order to solve the problem? For immediate feedback, you could add a length check directly in the text input's onChange handler. This check would not set any state but could display a warning message next to the input field in real-time. The debounced function would still be responsible for setting the state. |
📣 @zboutchyard! 📣
|
@JmillsExpensify, @abdulrahuman5196 Eep! 4 days overdue now. Issues have feelings too... |
@JmillsExpensify, @abdulrahuman5196 Huh... This is 4 days overdue. Who can take care of this? |
Sorry for the delay. Will review in my morning. |
Reviewing now |
@aswin-s 's proposal here #33612 (comment) looks good and works well. 🎀 👀 🎀 |
Triggered auto assignment to @tylerkaraszewski, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @abdulrahuman5196 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @aswin-s 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
PR got deployed to production a week ago. Melvin doesn't seem to bother. |
Not a regression.
Yes.
@JmillsExpensify Seems melvin didn't update this issue. Its pending payment now since the change was deployed to production 2 weeks ago. Added BZ checklist as well. |
@JmillsExpensify Gentle Reminder ☝️ |
Thanks for the DM! Looks like the automation failed. Payment summary:
|
All contracts paid out. As for the regression test, I'm not sure this is required. This is an inconvenience. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.17.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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
Action Performed:
Expected Result:
The error message should be visible without any delay.
Actual Result:
There's a delay for showing the "over 10 000" message again.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6326339_1703616047941.Over_the_limit_draft.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: