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

[HOLD for payment 2022-02-08] Chat - Unable to type next paragraph in message - Reported by: @Tushu17 #6767

Closed
mvtglobally opened this issue Dec 15, 2021 · 57 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@mvtglobally
Copy link

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:

  1. Go to app
  2. Open any chat
  3. Type 1 line & hit return to add a paragraph in same message

Expected Result:

In app user is able to add multiple paragraphs to the message by hitting return

Actual Result:

User is unable to add multiple lines in messages. When hit return, message is sent

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.1.20-2
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @Tushu17
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1639342107146300

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @jasperhuangg (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@jasperhuangg jasperhuangg added the External Added to denote the issue can be worked on by a contributor label Dec 16, 2021
@jasperhuangg jasperhuangg removed their assignment Dec 16, 2021
@MelvinBot
Copy link

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

@jasperhuangg
Copy link
Contributor

Ohh wait I'm dumb, this isn't even an issue and is expected behavior.

Use [Shift]+[Return] instead of just [Return] to add a newline.

Closing this out.

@Tushu17
Copy link
Contributor

Tushu17 commented Dec 16, 2021

Hello @jasperhuangg , It's for mobile web on mobile web we can't add a new line because there is no shift key on mobile.

@Tushu17
Copy link
Contributor

Tushu17 commented Dec 24, 2021

Bump @jasperhuangg

@jasperhuangg jasperhuangg removed their assignment Dec 24, 2021
@jasperhuangg
Copy link
Contributor

Should be good to work on by an external contributor

@Tushu17
Copy link
Contributor

Tushu17 commented Dec 27, 2021

@jasperhuangg I think you forgot to reopen this issue.

@Tushu17
Copy link
Contributor

Tushu17 commented Jan 6, 2022

Bump @jasperhuangg

@jasperhuangg jasperhuangg reopened this Jan 6, 2022
@jasperhuangg
Copy link
Contributor

jasperhuangg commented Jan 6, 2022

Ahh I finally understand what you mean @Tushu17

Next time it would be helpful to include a video recording with the issue to prevent confusion.

Upload.from.GitHub.for.iOS.MOV

I see that this behavior doesn't happen for native iOS (hitting return gives you a new line). Are we going to change the behavior of mobile web to match that of native platforms?

@MitchExpensify
Copy link
Contributor

Upwork

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

@parasharrajat
Copy link
Member

Sounds like a plan.

@ahmdshrif
Copy link
Contributor

ahmdshrif commented Jan 20, 2022

looking good. @roryabraham
@parasharrajat but looks like we have an issue with virtualkeyboard API.
it's not working with me on the project (give undefined value )

but I run the same code on expo snack it works in the same browser(updated chrome on mac and android ).

I do the test from the console
again it's not working on our project from console
but work on other websites consoles(Github for ex).

I think we need to update something to be able to use this API.
or it's a local host issue.

    isVirtualKeyboardOpen() {
        if ('virtualkeyboard' in navigator) {
            console.log('virtualkeyboard is supported!!');
            const { x, y, width, height } = navigator.virtualKeyboard.boundingRect;
            console.log('Virtual keyboard geometry:', x, y, width, height);
            return y > 0;
        }
        return canUseTouchScreen();
    }

@ahmdshrif
Copy link
Contributor

ahmdshrif commented Jan 20, 2022

I test the console for https://new.expensify.com/ and it works
looks like it's a localhost issue. do we have a test domain to try it?

@parasharrajat
Copy link
Member

It is virtualKeyboard not virtualkeyboard.

@ahmdshrif
Copy link
Contributor

yes, I do the test it without this condition (just write it for this on the comment.)

that is the navigator object I get not have virtualKeyboard .

appCodeName: "Mozilla"
appName: "Netscape"
appVersion: "5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/97.0.4692.99 Safari/537.36"
connection: NetworkInformation {onchange: null, effectiveType: '4g', rtt: 150, downlink: 3.3, saveData: false}
cookieEnabled: true
doNotTrack: null
geolocation: Geolocation {}
hardwareConcurrency: 12
ink: Ink {}
language: "en-GB"
languages: (5) ['en-GB', 'ar-EG', 'ar', 'en-US', 'en']
maxTouchPoints: 0
mediaCapabilities: MediaCapabilities {}
mediaSession: MediaSession {metadata: null, playbackState: 'none'}
mimeTypes: MimeTypeArray {0: MimeType, 1: MimeType, application/pdf: MimeType, text/pdf: MimeType, length: 2}
onLine: true
pdfViewerEnabled: true
permissions: Permissions {}
platform: "MacIntel"
plugins: PluginArray {0: Plugin, 1: Plugin, 2: Plugin, 3: Plugin, 4: Plugin, PDF Viewer: Plugin, Chrome PDF Viewer: Plugin, Chromium PDF Viewer: Plugin, Microsoft Edge PDF Viewer: Plugin, WebKit built-in PDF: Plugin,}
product: "Gecko"
productSub: "20030107"
scheduling: Scheduling {}
userActivation: UserActivation {hasBeenActive: true, isActive: true}
userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/97.0.4692.99 Safari/537.36"
vendor: "Google Inc."
vendorSub: ""
webdriver: false
webkitPersistentStorage: DeprecatedStorageQuota {}
webkitTemporaryStorage: DeprecatedStorageQuota {}

maybe it's a local issue for me since, in your production, I can get virtualKeyboard from navigator in the console.
wherever we still have the second check for touch screen implemented if the browser does not support it.

@parasharrajat
Copy link
Member

parasharrajat commented Jan 20, 2022

Yeah, Firefox does not support this API yet. Global coverage is 66%

@ahmdshrif
Copy link
Contributor

ahmdshrif commented Jan 20, 2022

It's chrome browser but idk why it's log Firefox!.

Looklike i have local issue . But we can going with that solution .

update : it's the default value .

The value of the Navigator.appCodeName property is always "Mozilla", in any browser. This property is kept only for compatibility purposes.

https://user-agents.net/string/mozilla-5-0-macintosh-intel-mac-os-x-10-15-0-applewebkit-537-36-khtml-like-gecko-chrome-78-0-3904-97-safari-537-36
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/appCodeName

@roryabraham
Copy link
Contributor

Okay, thanks for the updates here. @ahmdshrif Can you please write a single, cohesive proposal for this issue that I can evaluate before we move forward?

@ahmdshrif
Copy link
Contributor

ahmdshrif commented Jan 20, 2022

@roryabraham yes that is my final one : -

 isVirtualKeyboardOpen() {
    if ('virtualkeyboard' in navigator) {
        const  keybordPostion = navigator.virtualKeyboard.boundingRect.y;
        return keybordPostion > 0;
    }
    return canUseTouchScreen();
}
triggerHotkeyActions(e) {
  if (!e || this.isVirtualKeyboardOpen()) {
    return;
  }
...
}

in Constructor

this.checkVirtualkeyboard = this.isVirtualkeyboardOpen.bind(this);

@parasharrajat
Copy link
Member

@ahmdshrif This does not work.

if ('virtualkeyboard' in navigator)

Should run only on the web.

@ahmdshrif
Copy link
Contributor

ahmdshrif commented Jan 20, 2022

I mean: -
1- if we are on the web and we have virtualkeyboard support we will use it to know if the virtual keyboard is on or of.

Native(android and ios) also has navigator object without virtualkeyboard so this check is valid for all platforms .
but I make also check if there is a case in native navigator can be undefined.

  if (navigator && 'virtualkeyboard' in navigator) {
        // this web app or Desktop  with support  `virtualKeyboard` . so we can track if keyboard Is on or of 
        const  keybordPostion = navigator.virtualKeyboard.boundingRect.y;
        return keybordPostion > 0; // keyboard is open if there position more than 0
}

2- if we are in the native or unsupported browser we will check if this device has a touch screen.
return

    // all native platform  or web not support `virtualkeyboard`
    return canUseTouchScreen();

In the end, as I explain here #6767 (comment) the current code is disable this behavior from the native platform with this way.

so no need to add something like if (os.platform !== web) return

@parasharrajat
Copy link
Member

OK sounds good to me.

cc: @roryabraham

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 20, 2022
@MelvinBot
Copy link

📣 @ahmdshrif You have been assigned to this job by @roryabraham!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@ahmdshrif
Copy link
Contributor

ok, I apply to Upwork. working on PR

cc: @MitchExpensify

@MitchExpensify
Copy link
Contributor

Hired @ahmdshrif & @parasharrajat for eventual C+ payment!

@roryabraham roryabraham added the Reviewing Has a PR in review label Jan 26, 2022
@Tushu17
Copy link
Contributor

Tushu17 commented Jan 26, 2022

@MitchExpensify, can you hire me for reporting bonus. I have submitted the proposal but it's not showing up for some reason

@MitchExpensify
Copy link
Contributor

Offer sent @Tushu17 !

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Feb 1, 2022
@botify
Copy link

botify commented Feb 1, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.33-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 2022-02-08. 🎊

@botify botify changed the title Chat - Unable to type next paragraph in message - Reported by: @Tushu17 [HOLD for payment 2022-02-08] Chat - Unable to type next paragraph in message - Reported by: @Tushu17 Feb 1, 2022
@ahmdshrif
Copy link
Contributor

hi, @MitchExpensify I get the submit request rejected last week. do we have an issue here?

@MitchExpensify
Copy link
Contributor

@ahmdshrif paid for the fix and contract ended (That "rejection" probably happened automatically for some reason e.g. the job closing automatically - All good now!)
@parasharrajat paid for C+ and contract ended
@Tushu17 paid for reporting and contract ended

Closing!

@ahmdshrif
Copy link
Contributor

Ok thanks

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 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants