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

#2412 <Enter> key issue on mobile #2413

Merged
merged 6 commits into from
Nov 26, 2024

Conversation

SebinSong
Copy link
Collaborator

closes #2412

I can write multi-line messages from mobile now.

@SebinSong SebinSong self-assigned this Nov 6, 2024
Copy link

cypress bot commented Nov 6, 2024

group-income    Run #3438

Run Properties:  status check passed Passed #3438  •  git commit 558ff5324b ℹ️: Merge b171eab23a4865edd1c9904e4f1da75e89f58de3 into 05835cbc82dd789b9868e0659035...
Project group-income
Branch Review sebin/task/#2412-enter-key-issue-on-mobile
Run status status check passed Passed #3438
Run duration 08m 53s
Commit git commit 558ff5324b ℹ️: Merge b171eab23a4865edd1c9904e4f1da75e89f58de3 into 05835cbc82dd789b9868e0659035...
Committer Sebin Song
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 111
View all changes introduced in this branch ↗︎

@corrideat
Copy link
Member

corrideat commented Nov 14, 2024

LGTM.

I'm not so happy about the mobile detection, but then I can't really find a better way of doing this. What I'd do is maybe add a setting or a UI component (like a checkbox or dropdown) to explicitly select the Enter behaviour (tri-state: 'auto', ⇧ Shift+Enter for new line, Enter for new line) but that may be beyond the scope of this issue.

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @SebinSong!

I found one minor concern re Vue.js behavior (see comment), and a bug on mobile: try typing some text, and then pressing Enter many times on mobile. What will happen is that the text field will get taller up to a point, and the original text you typed will get moved way up. However, you won't be able to scroll the text field to edit the original text you type. On Android at least, whenever I tried to scroll the textfield, it would instead scroll the background chat.

const mediaIsPhone = window.matchMedia('(hover: none) and (pointer: coarse)')
this.ephemeral.isPhone = mediaIsPhone.matches
mediaIsPhone.onchange = (e) => { this.ephemeral.isPhone = e.matches }
this.mediaIsPhone = window.matchMedia('(hover: none) and (pointer: coarse)')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be creating a variable on the Vue object on-the-fly, which, as far as I know, isn't recommended by Vue.js. So correct me if I'm wrong, but I think this mediaIsPhone should be defined in the object returned by the data() section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it.

@SebinSong
Copy link
Collaborator Author

SebinSong commented Nov 26, 2024

@taoeffect

a bug on mobile: try typing some text, and then pressing Enter many times on mobile. What will happen is that the text field will get taller up to a point, and the original text you typed will get moved way up. However, you won't be able to scroll the text field to edit the original text you type.

I've been trying to figure this out for a while thinking it's a bug too, but it appears that this is a default behaviour by the browsers on mobile. Also, this only happens when the keyboard panel is revealed. When you click outside the <textarea /> to fold the keyboard back and try scrolling again it does work.

For example, you can confirm this symptom happens in any <textarea /> elements in this MDN article too. (I've tried this on a web-app of my other company in NZ too and confirmed the same there.)

But we can still use below handle that is attached to the cursor to move up/down which allows us to push the text content back down to a position you want. I guess this is how we are supposed to scroll the text content on textarea while the keyboard panel is up on mobile.

Thanks,

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@taoeffect taoeffect merged commit fac9a1b into master Nov 26, 2024
4 checks passed
@taoeffect taoeffect deleted the sebin/task/#2412-enter-key-issue-on-mobile branch November 26, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<Enter> / <Return> key should create a new line instead of sending message
3 participants