Skip to content
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

[Awaiting Payment 2024-01-30] [$2000] We can send an empty message using dictation on iPhone #18121

Closed
1 of 6 tasks
kavimuru opened this issue Apr 28, 2023 · 90 comments
Closed
1 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Apr 28, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open App
  2. Go to any chat
  3. Press on the message composer
  4. Press the mic icon from the keyboard and do not speak anything
  5. Press the mic icon again to stop dictation

Expected Result:

If we have not spoken anything then send button should have to be disabled

Actual Result:

Send button is enabled on an empty message and we can send the empty message in the chat

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.7
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

dictate-bug.MP4
VSSR0553.1.MP4

Expensify/Expensify Issue URL:
Issue reported by: @jayeshmangwani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1682627289846559

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fe24292fbc44e5d8
  • Upwork Job ID: 1651956178824339456
  • Last Price Increase: 2023-05-08
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 28, 2023
@MelvinBot
Copy link

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@MelvinBot
Copy link

MelvinBot commented Apr 28, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@hungvu193
Copy link
Contributor

hungvu193 commented Apr 28, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

We can send an empty message using dictation on iPhone

What is the root cause of that problem?

We are checking if comment is empty to disable the send button, however the result return from empty dictation in iPhone was a escape character \ufffc, that's why our regex didn't work and button still can be enable.

isCommentEmpty: !!newComment.match(/^(\s)*$/),

What changes do you think we should make in order to solve the problem?

Add more condition to our isCommentEmpty's regex to prevent such thing. So our regex will look like:

isCommentEmpty: !!newComment.match(/^(\s|\ufffc)*$/),

What alternative solutions did you explore? (Optional)

None.

Result:

Screen.Recording.2023-04-28.at.10.08.56.mov

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Apr 28, 2023
@trjExpensify
Copy link
Contributor

I can reliably reproduce this. Moving on External.

@melvin-bot melvin-bot bot changed the title We can send an empty message using dictation on iPhone [$1000] We can send an empty message using dictation on iPhone Apr 28, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01fe24292fbc44e5d8

@MelvinBot
Copy link

Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 28, 2023
@MelvinBot
Copy link

Triggered auto assignment to @flodnv (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@MelvinBot
Copy link

📣 @DmytroDumas! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Screen Shot 2022-11-16 at 4 42 54 PM

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@hellohublot
Copy link
Contributor

hellohublot commented Apr 28, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

We can send an empty message using dictation on iPhone

What is the root cause of that problem?

When we start using the dictation on the keyboard, the system will automatically generate a _UITextPlaceholderAttachment, when the speech recognition ends, the system will delete _UITextPlaceholderAttachment and replace it with the recognized text, but when we pass _UI TextPlaceholderAttachment to Javascript , we will force convert _UITextPlaceholderAttachment object to \uFFFC, so after the system speech recognition is completed, the _UITextPlaceholderAttachment cannot be found.

What changes do you think we should make in order to solve the problem?

Solution A:
We can simply ignore \uFFFC in javascript, and the system will automatically delete this character after the speech recognition is completed

- onChangeText={comment => this.updateComment(comment, true)}
+ onChangeText={comment => /\uFFFC+/.test(comment) ? null : this.updateComment(comment, true)}

Solution B:
We can merge facebook/react-native#37188 into Expensify/react-native, then we modify

- disabled={isBlockedFromConcierge || this.props.disabled}
+ disabled={isBlockedFromConcierge || this.props.disabled || /\uFFFC+/.test(this.state.value)}
I recommend solution A more, because we don't want to change the `disable` of all send buttons

@svendev2024
Copy link

React Native is a powerful tool for developing cross-platform mobile apps, and I have extensive experience in working with it. I have encountered similar issues in the past and am confident that I have the skills and expertise to resolve this for you.

To fix the issue, we need to disable the send button if the user has not spoken any message. We can achieve this by using the onStartListening and onEndListening methods provided by the dictation library. These methods allow us to track the state of the dictation session, and we can use this information to enable/disable the send button accordingly.

Also we can fix as follow code:
onChangeText={comment => this.updateComment(comment.replace(/\uFFFC/g, ''), true)}

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01fe1403955a8aad55

@MelvinBot
Copy link

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@melvin-bot melvin-bot bot added the Overdue label May 1, 2023
@Santhosh-Sellavel
Copy link
Collaborator

Quick thoughts

Seems the @hellohublot's proposal is straight forward one here

We should address this at all places, not just here

We can even do all the replacements in BaseTextInput, because they should not be sent to other platforms

cc: @flodnv

@melvin-bot melvin-bot bot removed the Overdue label May 1, 2023
@flodnv
Copy link
Contributor

flodnv commented May 1, 2023

Thanks @Santhosh-Sellavel. This is strange...

the system always add a \uFFFC at the end of the text

@hellohublot are we sure this is due to the OS and not React Native? This StackOverflow post may suggest that yes, it's the OS?

@hellohublot
Copy link
Contributor

hellohublot commented May 2, 2023

@flodnv @Santhosh-Sellavel
Thanks.

I investigated further and it is indeed a React Native issue.
I updated my proposal and the root cause, I also submitted a PR to the upstream. and I would prefer Solution A

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented May 3, 2023

@hellohublot I'm not sure

/\uFFFC+/ Having this check on multiple places is a bad idea, we need to handle this at a single place for addressing this at every inputs

@hellohublot
Copy link
Contributor

@Santhosh-Sellavel Thanks

Thanks, but the following video looks like we don’t need to change the disabled state. When every button is clicked, the system will interrupt the dictation first, then update the text, and then respond to the button click, so we only need to wait for the Upstream PR to be merged, and then update version or merge it into Expensify/react-native

_after.mp4

@hellohublot
Copy link
Contributor

@flodnv It seems that another issue will be solved too, both are iOS16+ to reproduce, I tested both can be solved

Would you mind holding the other one (it doesn't have a proposal yet), or raising the bounty on this one, because it did cost me some time and I didn't find any solution through google

@melvin-bot melvin-bot bot removed the Overdue label Dec 18, 2023
@melvin-bot melvin-bot bot added the Overdue label Jan 18, 2024
@flodnv
Copy link
Contributor

flodnv commented Jan 19, 2024

#31558 is on prod!!! 😱 Can someone retest this on iPhone please? 🙏

@melvin-bot melvin-bot bot removed the Overdue label Jan 19, 2024
@flodnv flodnv changed the title [HOLD React Native release] [$2000] We can send an empty message using dictation on iPhone [$2000] We can send an empty message using dictation on iPhone Jan 19, 2024
@flodnv flodnv added Weekly KSv2 and removed Monthly KSv2 labels Jan 19, 2024
@Santhosh-Sellavel
Copy link
Collaborator

@flodnv It's fixed now, tested now.

1.4.24 Issue Exists

Before_iOS.mp4

1.4.28 Works Fine

Latest.Version.mp4

@flodnv
Copy link
Contributor

flodnv commented Jan 19, 2024

Hooray!!!! 😍

@trjExpensify finally we can issue payments here!!! 💸

Once again, thanks for fixing this @hellohublot 👍

@trjExpensify
Copy link
Contributor

Niceeee! Okay, so confirming these are the payments?

@trjExpensify trjExpensify added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 30, 2024
@trjExpensify trjExpensify changed the title [$2000] We can send an empty message using dictation on iPhone [Awaiting Payment 2024-01-30] [$2000] We can send an empty message using dictation on iPhone Jan 30, 2024
@trjExpensify
Copy link
Contributor

Thanks for confirming, Flo.

@Santhosh-Sellavel - go ahead and request.
@hellohublot - offer sent
@jayeshmangwani - offer sent

Putting this on Daily and Awaiting payment to float it higher in my priority list :)

@trjExpensify
Copy link
Contributor

@hellohublot - paid!

@trjExpensify
Copy link
Contributor

@jayeshmangwani - paid!

@Santhosh-Sellavel once you confirm you've requested, I'll go ahead and close this.

@melvin-bot melvin-bot bot added the Overdue label Feb 1, 2024
@trjExpensify
Copy link
Contributor

Bump @Santhosh-Sellavel! Confirm you've requested, please.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 1, 2024
@trjExpensify
Copy link
Contributor

Mic check 1, 2.. @Santhosh-Sellavel you there? 😅

@melvin-bot melvin-bot bot added Daily KSv2 and removed Overdue Daily KSv2 labels Feb 5, 2024
@Santhosh-Sellavel
Copy link
Collaborator

Will request this week. Feel free to close

@trjExpensify
Copy link
Contributor

Thanks!

@Santhosh-Sellavel
Copy link
Collaborator

Requested on ND

@JmillsExpensify
Copy link

$2,000 approved for @Santhosh-Sellavel based on the summary above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

10 participants