[PAID] [$500] Web - Composer send button is enabled even when the input contains only space #26433
Comments
kbecciv
added
Daily
|
Triggered auto assignment to @strepanier03 ( |
Bug0 Triage Checklist (Main S/O)
|
|
Proposal by: @bernhardoj ProposalPlease re-state the problem that we are trying to solve in this issue.The composer send button is enabled even when it contains only space. What is the root cause of that problem?We disable the send button if https://github.com/Expensify/App/blob/c16995257c738761206ea07645e863ee717ecbc0/src/pages/home/report/ReportActionCompose/ReportActionCompose.js#[…]8 Notice that we check using _.isEmpty which considers space only as not empty. This is different from what we do when typing something in the composer. https://github.com/Expensify/App/blob/c16995257c738761206ea07645e863ee717ecbc0/src[…]ages/home/report/ReportActionCompose/ComposerWithSuggestions.js What changes do you think we should make in order to solve the problem?This issue happens after the |
ProposalPlease re-state the problem that we are trying to solve in this issue.Composer send button is not disabled when typed text is only spaces What is the root cause of that problem?Send Button is only checking for disabled if field is empty or not. App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js Lines 452 to 455 in 64ff471 What changes do you think we should make in order to solve the problem?We can trim the comment and then update What alternative solutions did you explore? (Optional)N/A |
|
📣 @alifhaider! 📣
|
|
Contributor details |
|
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
ProposalPlease re-state the problem that we are trying to solve in this issue.Composer send button is not disabled when typed text is only spaces What is the root cause of that problem?We disable the send button if App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js Lines 452 to 455 in ec95ab0 This is the root cause of this issue. What changes do you think we should make in order to solve the problem?We should also check if the isCommentEmpty: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
selector: (comment) => _.isEmpty(comment) || !!comment.match(/^(\s)*$/),
},What alternative solutions did you explore? (Optional)We can also be resolved by adding a check in the useEffect(() => {
if (value.length === 0) {
return;
}
+ setIsCommentEmpty(value.trim().length === 0);
Report.setReportWithDraft(reportID, true);
}, []); |
strepanier03
added
the
External
melvin-bot
bot
changed the title
|
Job added to Upwork: https://www.upwork.com/jobs/~0148ad3caa7badc613 |
melvin-bot
bot
added
the
Help Wanted
|
Current assignee @strepanier03 is eligible for the External assigner, not assigning anyone new. |
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.The Composer send button is disabled correctly when entering spaces inside of a chat but it is incorrectly enabled when re-entering that same chat room What is the root cause of that problem?When loading the page initially, the send button's disabled state is set by the initial value of isCommentEmptyProp, which is set here using However, when the state is updating while chatting, the value is set by using regex. The issue is that there is no common helper function to check whether a comment is empty so two different implementations currently exist. What changes do you think we should make in order to solve the problem?I think we should create a common What alternative solutions did you explore? (Optional)Just update all locations to use the regex but this issue can happen again later if a new engineer doesn't know to use the regex. The common helper method should resolve that issue |
ProposalPlease re-state the problem that we are trying to solve in this issue.When we open a chat and type multiple spaces in the composer, the send button remains disabled. But if we switch chats and navigate back to the original chat, the send button becomes enabled, instead of remaining disabled. What is the root cause of that problem?The root cause of the problem is that the check used to determine if a comment is empty, uses the _.isEmpty() underscore function, which, for strings and array-like objects, checks if the length property is 0. This function doesn't account for white spaces within a string. The current check happens on 452 isCommentEmpty: {
453 key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
454 selector: (comment) => _.isEmpty(comment),
455 },What changes do you think we should make in order to solve the problem?I would propose to add a further check to determine if the empty comment has white spaces. I think that it would be useful to have a utility function that could be composed when doing the check. I was looking for a place on the basecode that might be appropriate for adding the utitlity function, and I thought of ReportUtil.js, but, without knowing the basecode in depth, I understand, there might be other files, where this utility function, might be more appropriately placed. I would suggest the following changes:
/**
* Checks if a comment has white space
*
* @param {String} comment
* @returns {Boolean}
*/
function hasWhiteSpace(comment) {
return /\s/g.test(comment);
} export {
...
hasWhiteSpace,
...
};
isCommentEmpty: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
selector: (comment) => _.isEmpty(comment) || ReportUtils.hasWhiteSpace(comment),
},What alternative solutions did you explore? (Optional)An alternative solution would be to inline the check for white spacing directly, instead of using a separate function. I thought that using a separate function might help with readability. |
|
📣 @rsiota! 📣
|
|
Contributor details |
|
|
|
@strepanier03, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick! |
|
Thanks everyone for the proposal. Looks like @bernhardoj's solution has a straightforward fix for this issue. Let's proceed with it. 🎀 👀 🎀 C+ reviewed |
|
Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
|
I'm not being able to assign @bernhardoj for the moment 
I was having some problems with Github crashing a few minutes ago, maybe is related to that, I'll try later. |
|
📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app! |
melvin-bot
bot
added
Reviewing
|
PR is ready cc: @fedirjh |
|
Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:
On to the next one 🚀 |
melvin-bot
bot
added
Weekly
melvin-bot
bot
changed the title
|
|
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.74-3 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-05. 🎊 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:
|
|
@bernhardoj - You've been paid in Upwork. I left a note on the bonus payment but I thought we had two separate contracts for this job for you so I added the bonus to pay the reporting payment after the fact. Let me know if you have any questions. @fedirjh - I'll keep my eyes open for an update on the checklist and move forward once there's one. Thanks everyone! |
|
@strepanier03 got it. Thanks! I think you can close the Reporter contract then. |
BugZero Checklist:
Regression Test Proposal
|
|
@strepanier03 The offer was expired for me, could you please send me another offer 🙏🏼 |
|
Sure thing @fedirjh, I'll do that now. |
|
@fedirjh - Sent! Working on reg test GH now. |
|
@bernhardoj - I'll work on closing that next week. For some reason both contracts linked here are for the same contract and paid already. |
|
@strepanier03 Accepted, Thank you. |
|
@fedirjh - I have finished paying you as well. Thanks for accepting that. Thank you both for being part of our community and working so hard to help us achieve the New Expensify dream. |
strepanier03
changed the title
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 send button stays disabled because it contains only space
Actual Result:
The send button is enabled
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.60.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
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2023-08-26.at.16.06.42.mov
Recording.4142.mp4
Expensify/Expensify Issue URL:
Issue reported by: @bernhardoj
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693037242762969
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: