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

Send typing notification #2302

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Send typing notification #2302

merged 1 commit into from
Jan 29, 2024

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Jan 26, 2024

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Send typing notification.
Note: there will be a setting to disable typing notification, it's tracked by #2241.

Motivation and context

Closes #2240

Screenshots / GIFs

NA

Tests

  • Start typing in a room and observe with another session and using a client which supports rendering of typing notification that the typing notification is displayed.

Tested devices

  • Physical
  • Emulator + Ele Web.
  • OS version(s):

Checklist

@bmarty bmarty requested a review from a team as a code owner January 26, 2024 15:34
@bmarty bmarty requested review from jmartinesp and removed request for a team January 26, 2024 15:34
Copy link

sonarcloud bot commented Jan 26, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/Ai9rJC

Copy link
Member

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

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

The changes LGTM, and I'm glad you found the onTyping API of the RTE!

There's only one thing that bothers me, the Rust SDK doesn't seem to be sending a 'no longer typing' message to the server when we set call typingNotice(false). If I set up the debugger I can see the false value arriving and being sent to the Rust SDK, but when I look at the other client I can still see the typing... status until the 4s timeout happens.

It's not something we can fix in this PR, so we should merge it as is, but I think we should also investigate what's wrong with the SDK behaviour.

@bmarty bmarty added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Jan 26, 2024
@github-actions github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Jan 26, 2024
@bmarty bmarty merged commit 22defe5 into develop Jan 29, 2024
17 checks passed
@bmarty bmarty deleted the feature/bma/sendTyping branch January 29, 2024 08:16
@bmarty
Copy link
Member Author

bmarty commented Jan 29, 2024

matrix-org/matrix-rust-sdk#3064 has been created to track the observed issue.

@bmarty
Copy link
Member Author

bmarty commented Feb 9, 2024

There's only one thing that bothers me, the Rust SDK doesn't seem to be sending a 'no longer typing' message to the server when we set call typingNotice(false). If I set up the debugger I can see the false value arriving and being sent to the Rust SDK, but when I look at the other client I can still see the typing... status until the 4s timeout happens.

Actually I think it's a feature of Element Web; see matrix-org/matrix-rust-sdk#3064 (comment)

If you use another client, for instance Element Android, the typing notification disappears faster when the other user stops typing.

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.

[Task] Send typing notification
2 participants