element-android
element-android copied to clipboard
Create the DM when sending an event
Type of change
- [X] WIP Feature
- [ ] Bugfix
- [ ] Technical
- [ ] Other :
Content
Depends on #6051
Create the DM room when an event is sent from the local room timeline (text, image, audio, poll, location sharing...)
Motivation and context
Continue #5525
Screenshots / GIFs
Sender | Receiver | |
---|---|---|
By text | https://user-images.githubusercontent.com/11990514/178769378-f9348056-c725-418a-a830-f0afbad358bd.mp4 | https://user-images.githubusercontent.com/11990514/178769798-f7f90ce8-d410-47c2-9a07-672bd4278e06.mp4 |
By image | https://user-images.githubusercontent.com/11990514/178770472-850fa810-aaa2-41b9-9a81-989a96da9d95.mp4 | https://user-images.githubusercontent.com/11990514/178770509-30da044d-4b56-4f25-aa8e-fb78d29d5aa8.mp4 |
By location | https://user-images.githubusercontent.com/11990514/178770654-7bef2d98-33a0-4eef-bae9-5682bede97f1.mp4 | https://user-images.githubusercontent.com/11990514/178770719-80cb39ec-231b-4d51-bedb-00282632f600.mp4 |
Tests
- Open a local room timeline (by clicking on the fab button from the DM room list, selecting one or several users, and pressing the "GO" button)
- Send an event from the composer view (message, image or anything else)
- The DM room should be created, the selected users should be invited and we can continue our discussion in that room.
- Press the back button should redirect to the home screen, the local room is not displayed anymore.
Tested devices
- [ ] Physical
- [X] Emulator
- OS version(s): API 32
Checklist
- [ ] Changes has been tested on an Android device or Android emulator with API 21
- [ ] UI change has been tested on both light and dark themes
- [ ] Accessibility has been taken into account. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
- [X] Pull request is based on the develop branch
- [X] Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
- [x] Pull request includes screenshots or videos if containing UI changes
- [ ] Pull request includes a sign off
- [x] You've made a self review of your PR
- [ ] If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()
Unit Test Results
124 files ±0 124 suites ±0 2m 17s :stopwatch: -5s 220 tests ±0 220 :heavy_check_mark: ±0 0 :zzz: ±0 0 :x: ±0 726 runs ±0 726 :heavy_check_mark: ±0 0 :zzz: ±0 0 :x: ±0
Results for commit 5c078bd1. ± Comparison against base commit 521ecfb6.
:recycle: This comment has been updated with latest results.
Cool feature! I added some suggestions mostly to ease reading of code. Let me know what do you think about it. I didn't test it but seeing the videos of the PR it looks good.
@mnaturel Thanks for your review! I totally agree about adding tests, I didn't mention it in the PR description but I'll add some unit tests, I would do it in a dedicated PR but I'll see if I can add some in this PR. BTW it's good to have your feedback on which part of code you suggest adding tests!
Thanks for the update. I have tested the feature, nomical case, i.e. inviting a matrixId and sending a text message, and this is working pretty well. 2 small glitches observed: first one on the timeline, when the room is created. 2nd one when going back to the room list, I can see twice the rooms for a few ms. Nothing blocking though!
Approved!
Thanks for your feedback!
The first glitch should be fixed by adding a loading wheel during the creation but I did not find a way to do it atm. The second one is more annoying because the local rooms are deleted when displaying the room list, I can try to do the deletion as soon as the new room has been created, but not sure it will fix the glitch