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

implement emotes (/me) #1841

Merged
merged 8 commits into from Oct 3, 2023
Merged

implement emotes (/me) #1841

merged 8 commits into from Oct 3, 2023

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Oct 1, 2023

requires matrix-org/matrix-rust-sdk#2648

closes #1107

Arguably this could be handled by a slash-command engine in matrix-sdk-ui, but given it's not clear EX is going to even have slash commands (given whatsapp & imessage & signal etc don't), I'm adding it as a quick & dirty easter egg in the application layer. Given the logic is so simple, I'm not convinced it's worth the complexity of shoving it into the SDK at this point.

Handles:

  • plaintext/markdown msg which begins /me
  • richtext editor msg whose plaintext/markdown representation begins /me
  • editing existing plaintext msgs
  • editing existing richtext msgs
  • replying with a /me (plaintext)
  • replying with a /me (richtext)

Pull Request Checklist

@ara4n ara4n requested a review from a team as a code owner October 1, 2023 23:52
@ara4n ara4n requested review from pixlwave and removed request for a team October 1, 2023 23:52
@github-actions
Copy link

github-actions bot commented Oct 1, 2023

Warnings
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ Please add a sign-off to either the PR description or to the commits themselves.

Generated by 🚫 Danger Swift against 984e9a5

@ara4n
Copy link
Member Author

ara4n commented Oct 2, 2023

How do I make the tests run against the right sdk branch name? (EW would at least automatically try to build against the branch of the same name in the SDK...)

@stefanceriu stefanceriu requested review from Velin92 and removed request for pixlwave October 2, 2023 05:53
Copy link
Contributor

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

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

Left comments inline for a couple of small tweaks that need to happen. Otherwise looks sensible to me.

@Velin92 you were going to look into user suggestions this week. They should probably be based on CompletionSuggestion from the old codebase which also supports slash commands so please take this PR under your wing.

ElementX/Sources/Services/Room/RoomProxy.swift Outdated Show resolved Hide resolved
ElementX/Sources/Services/Room/RoomProxy.swift Outdated Show resolved Hide resolved
@stefanceriu
Copy link
Contributor

run against the right sdk branch name?

There is nothing like that atm. We agreed to look into building the sdk on the CI after the next release and this can very well be a part of that work.

@manuroe
Copy link
Member

manuroe commented Oct 2, 2023

run against the right sdk branch name?

There is nothing like that atm. We agreed to look into building the sdk on the CI after the next release and this can very well be a part of that work.

We will also need to improve the infra. GH action runners are slow to build the SDK bindings. They take 4h to build them for Android. This would be way too long to validate a PR on EAX or EIX.

@ara4n
Copy link
Member Author

ara4n commented Oct 2, 2023

ptal @stefanceriu

@stefanceriu stefanceriu enabled auto-merge (squash) October 3, 2023 12:45
@stefanceriu stefanceriu enabled auto-merge (squash) October 3, 2023 12:46
@sonarcloud
Copy link

sonarcloud bot commented Oct 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Attention: 42 lines in your changes are missing coverage. Please review.

Comparison is base (e6fa1ba) 71.49% compared to head (984e9a5) 71.31%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1841      +/-   ##
===========================================
- Coverage    71.49%   71.31%   -0.19%     
===========================================
  Files          442      442              
  Lines        30058    30083      +25     
  Branches     14738    14748      +10     
===========================================
- Hits         21490    21453      -37     
- Misses        8031     8093      +62     
  Partials       537      537              
Flag Coverage Δ
unittests 54.83% <0.00%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...meline/TimelineItems/RoomTimelineItemFactory.swift 20.94% <0.00%> (ø)
...urces/Screens/RoomScreen/RoomScreenViewModel.swift 55.76% <0.00%> (-0.43%) ⬇️
ElementX/Sources/Services/Room/RoomProxy.swift 24.77% <0.00%> (-0.73%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stefanceriu stefanceriu merged commit 68ec7bb into develop Oct 3, 2023
10 checks passed
@stefanceriu stefanceriu deleted the matthew/emotes branch October 3, 2023 13:17
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.

Add support for /me command
4 participants