Start DM on first message #6367

yostyle · 2022-07-01T17:48:51Z

#5864

sonarcloud · 2022-07-01T17:50:24Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

codecov-commenter · 2022-07-01T18:33:40Z

Codecov Report

Merging #6367 (aa1ec2d) into develop (33540c9) will increase coverage by 0.05%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #6367      +/-   ##
===========================================
+ Coverage     6.13%    6.19%   +0.05%     
===========================================
  Files         1475     1476       +1     
  Lines       156165   156546     +381     
  Branches     62571    62735     +164     
===========================================
+ Hits          9577     9691     +114     
- Misses      146192   146451     +259     
- Partials       396      404       +8

Impacted Files	Coverage Δ
Riot/Generated/Strings.swift	`2.85% <0.00%> (-0.01%)`	⬇️
Riot/Modules/Application/LegacyAppDelegate.m	`13.34% <0.00%> (+0.61%)`	⬆️
...on/ScreenNavigation/RoomNavigationParameters.swift	`0.00% <0.00%> (ø)`
...es/Contacts/Details/ContactDetailsViewController.m	`0.00% <0.00%> (ø)`
...ot/Modules/Home/AllChats/AllChatsCoordinator.swift	`0.00% <0.00%> (ø)`
...dules/MatrixKit/Views/RoomTitle/MXKRoomTitleView.m	`0.00% <0.00%> (ø)`
...ules/Room/Members/RoomParticipantsViewController.m	`0.00% <0.00%> (ø)`
Riot/Modules/Room/RoomCoordinator.swift	`0.00% <0.00%> (ø)`
Riot/Modules/Room/RoomCoordinatorParameters.swift	`0.00% <0.00%> (ø)`
Riot/Modules/Room/RoomViewController.m	`0.00% <0.00%> (ø)`
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2022-07-01T19:13:09Z

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/miYLKi

giomfo · 2022-07-04T20:24:31Z

@yostyle I observed a crash during my test

I got it by filling a wrong matrix id (not an actual account)

giomfo · 2022-07-04T20:34:26Z

In the empty room (temporary DM), the voice message is displayed in the text composer, but it seems disabled

I would replace the placeholder with "Send your first message" to satisfy a minimum the requested design.

Riot/Modules/Room/RoomCoordinator.swift

Riot/Modules/Room/RoomViewController.m

Riot/Modules/Room/RoomViewController.h

Riot/Modules/Room/RoomViewController.m

Riot/Modules/Room/RoomCoordinator.swift

Riot/Modules/Room/RoomViewController.m

Riot/Modules/Room/Views/InputToolbar/RoomInputToolbarView.h

Riot/Modules/Room/RoomViewController.h

Riot/Modules/Room/RoomViewController.m

Riot/Assets/de.lproj/Vector.strings

giomfo · 2022-08-17T16:56:23Z

@yostyle I did some tests

the placeholder "Send your first message" is not reset after the DM creation (the RoomInputToolbarViewSendMode has to be updated)
I created a second DM by sending a new message in this first one NOK

yostyle · 2022-08-19T15:53:46Z

@yostyle I did some tests

the placeholder "Send your first message" is not reset after the DM creation (the RoomInputToolbarViewSendMode has to be updated)

I created a second DM by sending a new message in this first one NOK

Fixed

giomfo · 2022-08-22T12:13:45Z

@ismailgulek Hi, except the following small remark, all the points already mentioned during our first (@phlniji and I) reviews have been fixed.
I will approve this PR, but we will wait for your review before being able to merge it. Is that ok for you?

giomfo

LGTM - tests ok

ismailgulek · 2022-08-22T13:23:41Z

@ismailgulek Hi, except the following small remark, all the points already mentioned during our first (@phlniji and I) reviews have been fixed.

I will approve this PR, but we will wait for your review before being able to merge it. Is that ok for you?

Yes, i'll provide a review when appropriate.

ismailgulek

I had some comment inline, otherwise looks sane to me.

Riot/Modules/Room/RoomCoordinator.swift

Riot/Modules/Room/RoomCoordinatorParameters.swift

Riot/Modules/Room/RoomViewController.m

Riot/Modules/Room/Views/InputToolbar/RoomInputToolbarView.m

Riot/Modules/Room/RoomCoordinator.swift

giomfo · 2022-08-25T13:19:13Z

@ismailgulek FYI I'm working with @phlniji to take into account your comment. We will ping you here when the PR will be ready for a new review

Riot/Modules/Room/RoomViewController.m

giomfo

second review : approved

giomfo · 2022-08-26T08:39:02Z

@ismailgulek FYI this PR has been updated, your review is requested on the last commit of @phlniji which should fixed all your comments

ismailgulek

Thanks for the changes, LGTM now. I just have new tiny question.

ismailgulek · 2022-08-26T10:04:30Z

Riot/Modules/Room/RoomCoordinator.swift

@@ -80,6 +80,9 @@ final class RoomCoordinator: NSObject, RoomCoordinatorProtocol {
        if let threadId = parameters.threadId {
            self.roomViewController = ThreadViewController.instantiate(withThreadId: threadId,
                                                                       configuration: parameters.displayConfiguration)
+        } else if parameters.userId != nil {


Is this really needed? I think the view controller will be created with right parameters even if we remove this.

Thx for your final review
I agree with you, the session seems provided latter to the view controller (via the - (void)displayNewDirectChatWithTargetUser:(nonnull MXUser*)directChatTargetUser session:(nonnull MXSession*)session method)
I will check this point with @yostyle to have a third point of view before cleaning the code here

You right session parameter is not needed. I removed it

giomfo · 2022-08-29T14:02:04Z

A final UX review based on build is planned before merging this PR

giomfo · 2022-08-29T15:29:32Z

During UX Review, we observed an issue: the selected attachment is ignored, it is not sent in the created DM

Clean code

giomfo

Approved

Note: In the last commit we retrieve (in the createOrRestoreDiscussionIfNeeded completion block) the actual input tool bar after the potential DM creation. The input tool bar may have been replaced by this method

giomfo

@yostyle I did some additional tests by enabling the new Layout
The empty DM is not displayed correctly when I try to start a new DM
I got only a white screen (no header, no input tool bar...)
Must be fixed before merging

giomfo

Tests with the new layout succeeded

sonarcloud · 2022-09-06T12:02:30Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

yostyle requested a review from giomfo July 1, 2022 17:49

giomfo self-assigned this Jul 5, 2022

giomfo reviewed Jul 5, 2022

View reviewed changes

yostyle force-pushed the yostyle/start_dm_first_msg branch 3 times, most recently from a8d0293 to 322b2f1 Compare August 8, 2022 11:23

yostyle marked this pull request as ready for review August 8, 2022 12:15

phloux reviewed Aug 10, 2022

View reviewed changes

Riot/Modules/Room/RoomViewController.m Show resolved Hide resolved

Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved

Riot/Modules/Room/RoomCoordinator.swift Outdated Show resolved Hide resolved

Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved

giomfo reviewed Aug 16, 2022

View reviewed changes

yostyle force-pushed the yostyle/start_dm_first_msg branch from 9cc6005 to 5eba402 Compare August 17, 2022 14:23

yostyle force-pushed the yostyle/start_dm_first_msg branch from 5eba402 to 411cda8 Compare August 19, 2022 15:52

yostyle requested review from giomfo, a team, phloux and ismailgulek and removed request for giomfo and a team August 19, 2022 20:26

giomfo removed their assignment Aug 22, 2022

giomfo approved these changes Aug 22, 2022

View reviewed changes

ismailgulek suggested changes Aug 23, 2022

View reviewed changes

giomfo reviewed Aug 25, 2022

View reviewed changes

Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved

Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved

phloux force-pushed the yostyle/start_dm_first_msg branch from dc213c8 to ac1dda5 Compare August 26, 2022 06:53

giomfo approved these changes Aug 26, 2022

View reviewed changes

giomfo requested a review from ismailgulek August 26, 2022 08:37

ismailgulek approved these changes Aug 26, 2022

View reviewed changes

giomfo assigned gaelledel Aug 29, 2022

gaelledel mentioned this pull request Aug 29, 2022

Create DM room only on first message #5864

Closed

5 tasks

yostyle force-pushed the yostyle/start_dm_first_msg branch from ac1dda5 to 8264d25 Compare September 5, 2022 14:30

yostyle and others added 9 commits September 5, 2022 19:33

Start DM on first message

75a2d65

Retrieve user

bb94cbe

Disable voice message feature

185ac21

Replace the placeholder with "Send your first message"

4791e78

Disable voice message

248fd70

Clean code

Fix DM room creation issue

1603012

Display an alert when the user to invite does not exist

15cd688

Fix PR comments

03aa581

fix the attachments sending on first DM

0483cd8

yostyle force-pushed the yostyle/start_dm_first_msg branch from 8264d25 to 0483cd8 Compare September 5, 2022 17:53

giomfo self-requested a review September 6, 2022 06:47

giomfo approved these changes Sep 6, 2022

View reviewed changes

giomfo requested changes Sep 6, 2022

View reviewed changes

Fix start DM starting from new all chats layout

aa1ec2d

giomfo approved these changes Sep 6, 2022

View reviewed changes

Remove session in room controller instanciation

01c6060

yostyle merged commit e56981b into develop Sep 6, 2022

yostyle deleted the yostyle/start_dm_first_msg branch September 6, 2022 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start DM on first message #6367

Start DM on first message #6367

yostyle commented Jul 1, 2022

sonarcloud bot commented Jul 1, 2022

codecov-commenter commented Jul 1, 2022 •

edited

Loading

github-actions bot commented Jul 1, 2022 •

edited

Loading

giomfo commented Jul 4, 2022

giomfo commented Jul 4, 2022 •

edited

Loading

giomfo commented Aug 17, 2022

yostyle commented Aug 19, 2022

giomfo commented Aug 22, 2022

giomfo left a comment

ismailgulek commented Aug 22, 2022

ismailgulek left a comment

giomfo commented Aug 25, 2022

giomfo left a comment

giomfo commented Aug 26, 2022

ismailgulek left a comment

ismailgulek Aug 26, 2022 •

edited

Loading

giomfo Aug 26, 2022

yostyle Sep 6, 2022

giomfo commented Aug 29, 2022 •

edited

Loading

giomfo commented Aug 29, 2022

giomfo left a comment

giomfo left a comment

giomfo left a comment

sonarcloud bot commented Sep 6, 2022

Start DM on first message #6367

Start DM on first message #6367

Conversation

yostyle commented Jul 1, 2022

sonarcloud bot commented Jul 1, 2022

codecov-commenter commented Jul 1, 2022 • edited Loading

Codecov Report

github-actions bot commented Jul 1, 2022 • edited Loading

giomfo commented Jul 4, 2022

giomfo commented Jul 4, 2022 • edited Loading

giomfo commented Aug 17, 2022

yostyle commented Aug 19, 2022

giomfo commented Aug 22, 2022

giomfo left a comment

Choose a reason for hiding this comment

ismailgulek commented Aug 22, 2022

ismailgulek left a comment

Choose a reason for hiding this comment

giomfo commented Aug 25, 2022

giomfo left a comment

Choose a reason for hiding this comment

giomfo commented Aug 26, 2022

ismailgulek left a comment

Choose a reason for hiding this comment

ismailgulek Aug 26, 2022 • edited Loading

Choose a reason for hiding this comment

giomfo Aug 26, 2022

Choose a reason for hiding this comment

yostyle Sep 6, 2022

Choose a reason for hiding this comment

giomfo commented Aug 29, 2022 • edited Loading

giomfo commented Aug 29, 2022

giomfo left a comment

Choose a reason for hiding this comment

giomfo left a comment

Choose a reason for hiding this comment

giomfo left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 6, 2022

codecov-commenter commented Jul 1, 2022 •

edited

Loading

github-actions bot commented Jul 1, 2022 •

edited

Loading

giomfo commented Jul 4, 2022 •

edited

Loading

ismailgulek Aug 26, 2022 •

edited

Loading

giomfo commented Aug 29, 2022 •

edited

Loading