[NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread) #1960

rafaelks · 2018-07-13T04:08:34Z

@RocketChat/ios

What's new in this PR

Swipe actions on the subscription cells using SwipeCellKit
Preview actions on 3D Touch
Use DifferenceKit for calculating diff and updating the subscriptions table view

Note: The implementation of swipe actions provided in UIKit has several bugs, and in its current state does not work well with our use case. SwipeCellKit also has some issues, working with DifferenceKit, but the simple workaround is that we don't use the .destructive style.

Closes #1518
Closes #2131

…_subscriptions.1518 [FIX] Theming on swipe actions

codecov · 2018-07-27T02:13:17Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@111f4c0). Click here to learn what that means.
The diff coverage is 19.92%.

@@            Coverage Diff             @@
##             develop    #1960   +/-   ##
==========================================
  Coverage           ?   35.04%           
==========================================
  Files              ?      387           
  Lines              ?    18110           
  Branches           ?        0           
==========================================
  Hits               ?     6346           
  Misses             ?    11764           
  Partials           ?        0

Impacted Files	Coverage Δ
Rocket.Chat/Managers/AnalyticsManager.swift	`59.03% <ø> (ø)`
...Chat/Models/Subscription/SubscriptionActions.swift	`0% <0%> (ø)`
...equests/Subscription/SubscriptionHideRequest.swift	`0% <0%> (ø)`
...uests/Subscription/SubscriptionUnreadRequest.swift	`0% <0%> (ø)`
...iews/Cells/Subscription/BaseSubscriptionCell.swift	`0% <0%> (ø)`
...at/Models/Subscription/UnmanagedSubscription.swift	`0% <0%> (ø)`
...quests/Subscription/SubscriptionLeaveRequest.swift	`0% <0%> (ø)`
...at/Views/Cells/Subscription/SubscriptionCell.swift	`0% <0%> (ø)`
Rocket.Chat/Theme/ThemeableViews.swift	`77.5% <100%> (ø)`
...ket.Chat/Controllers/Chat/ChatViewController.swift	`29.8% <7.14%> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 111f4c0...5653a2b. Read the comment docs.

…equest and generating inconsistency

filipealva · 2018-08-22T14:44:57Z

@Sameesunkaria just experienced the Table section header title sometimes gets improperly laid out issue.

The Conversations header sometimes is not drawn, and when we have another section, for example the unread one, it looks like all the conversations are unread because we only see the Unread header.

filipealva · 2018-08-23T13:57:18Z

Rocket.Chat/Controllers/Subscriptions/SubscriptionsList/SubscriptionsViewModel.swift

+        reloadNotificationToken = realm?.objects(Auth.self).observe({ [weak self] _ in
+            if self?.realm?.objects(Auth.self).count == 1 {
+                DispatchQueue.main.async {
+                    self?.reloadData?()


@Sameesunkaria this is to make sure we will have the Auth object accessible when first loading the SubscriptionsViewController?

This is to reload the cells so that the avatar views get updated when the server url is accessible. This is required after the first login. If you have a better solution, please let me know.

The solutions looks good, I was just thinking that we could invalidate the token after executing the block once. It seems that it only needs to be executed once. Does it makes sense?

It gets called a few times, so if we invalidate it before that (after the first time the block executes) it sometimes causes the images to not be loaded. I am not sure as to why it gets called more than once. If you can figure that out, maybe we'll be able to invalidate that notification.

Got it. Let's leave it this way, it's not critical, we can improve in another PR. Thanks for explaining

filipealva · 2018-08-23T17:37:16Z

Found one more bug related to section header layout

_{Sent with GitHawk}

…metimes we were not being able to get the correct center y in runtime

Sameesunkaria · 2018-08-23T17:42:11Z

Rocket.Chat/Views/Sections/SubscriptionSectionView.xib

                <constraint firstItem="gd8-ny-3Np" firstAttribute="trailing" secondItem="SwS-x7-2jR" secondAttribute="trailing" id="MYk-K7-viX"/>
                <constraint firstItem="SwS-x7-2jR" firstAttribute="trailing" secondItem="gOj-Q6-yQs" secondAttribute="trailing" constant="16" id="S8t-kq-5ZF"/>
-                <constraint firstItem="gOj-Q6-yQs" firstAttribute="centerY" secondItem="iN0-l3-epB" secondAttribute="centerY" id="ZmT-uy-R4W"/>
+                <constraint firstItem="gOj-Q6-yQs" firstAttribute="top" secondItem="SwS-x7-2jR" secondAttribute="top" constant="13.5" id="zGn-Dp-2U7"/>


Haha, was just about to push the same change...

Haha, cool! 🤓

filipealva

Everything working fine! Thank you guys for this PR, specially @Sameesunkaria 💯

cardoso

Great work overall, just a few considerations.

Only change I think is critical right now is to move the new Subscription methods to struct SubscriptionsClient

Rocket.Chat/Models/Subscription/UnmanagedSubscription.swift

cardoso · 2018-08-24T18:37:40Z

Rocket.Chat/Models/Subscription/SubscriptionActions.swift

+
+extension Subscription {
+    func favoriteSubscription() {
+        SubscriptionManager.toggleFavorite(self) { (response) in


These functions should be in struct SubscriptionsClient so we can use other APIs than API.current() and also be able to test with MockAPI.

@cardoso Made some changes...

Rocket.Chat/Models/User/UnmanagedUser.swift

Rocket.Chat/Models/Subscription/UnmanagedSubscription.swift

rafaelks · 2018-08-29T12:51:42Z

Rocket.Chat/Resources/pt-BR.lproj/Localizable.strings

+"subscriptions.list.actions.unread" = "Não lido";
+"subscriptions.list.actions.favorite" = "Favorite"; // TODO
+"subscriptions.list.actions.unfavorite" = "Unfavorite"; // TODO
+"subscriptions.list.actions.hide" = "Fechar";


"Esconder"? 🤔

rafaelks · 2018-08-29T12:51:59Z

Rocket.Chat/Resources/pt-BR.lproj/Localizable.strings

+"subscriptions.list.actions.read" = "Lido";
+"subscriptions.list.actions.unread" = "Não lido";
+"subscriptions.list.actions.favorite" = "Favorite"; // TODO
+"subscriptions.list.actions.unfavorite" = "Unfavorite"; // TODO


"Remover dos Favoritos"

This text is shown under the swipe actions. something shorter would be preferred.

rafaelks · 2018-08-29T12:52:07Z

Rocket.Chat/Resources/pt-BR.lproj/Localizable.strings

 "subscriptions.list.date.yesterday" = "Ontem";
+"subscriptions.list.actions.read" = "Lido";
+"subscriptions.list.actions.unread" = "Não lido";
+"subscriptions.list.actions.favorite" = "Favorite"; // TODO


"Adic. aos Favoritos"

"Adicionar aos Favoritos"? @rafaelks 🤔

This text is shown under the swipe actions. something shorter would be preferred.

rafaelks · 2018-08-29T14:49:16Z

Great work here @Sameesunkaria and @filipealva! Is this one ready to be merged?

Sameesunkaria · 2018-08-29T14:58:46Z

@rafaelks yes it is. Except the localization stuff.

_{Sent with GitHawk}

rafaelks added 2 commits July 13, 2018 01:04

Add the actions to the cell

a6fc957

Remove unused code

f3a54b7

rafaelks added the 🎉 feature label Jul 13, 2018

rafaelks added this to the 3.0.0 milestone Jul 13, 2018

rafaelks changed the title ~~[NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread)~~ [WIP][NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread) Jul 13, 2018

Fix theming on swipe actions

ff4a5ad

Sameesunkaria mentioned this pull request Jul 13, 2018

[FIX] Theming on swipe actions #1967

Merged

Sameesunkaria and others added 2 commits July 13, 2018 18:13

Theme autocompletion separator

53b4316

Merge pull request #1967 from Sameesunkaria/feature/implement_actions…

3ab9ae3

…_subscriptions.1518 [FIX] Theming on swipe actions

rafaelks modified the milestones: 3.0.0, 3.1.0 Jul 13, 2018

rafaelks assigned filipealva Jul 25, 2018

filipealva added 3 commits July 26, 2018 22:55

Fix merge conflicts

b62a90a

Add SubscriptionUnredRequest

3b456ff

Add request calls to unread/read actions

e493d85

filipealva added 9 commits July 27, 2018 15:16

Rely on alert property to handle mark as read/unread actions

f0fcac4

Add SubscriptionLeaveRequest

e752643

Don't add leave action for dms

09fc226

Implement SubscriptionLeaveRequest

6dd1b6b

Fix SubscriptionLeaveRequest

4d68504

Stop adding leave action for cases that it will wind up failing the r…

b0079e8

…equest and generating inconsistency

Add SubscriptionHideRequest

459ed8b

Implement SubscriptionHideRequest

207756a

Localize actions

088ef8c

rafaelks mentioned this pull request Aug 2, 2018

Messages can no longer be marked as unread #2102

Closed

Sameesunkaria added 4 commits August 5, 2018 19:58

Merge branch 'develop' into feature/implement_actions_subscriptions.1518

4db8d7a

Add favorites action

cf6344f

Using DifferenceKit - WIP

4b5582f

Fix crash

6db53ed

Sameesunkaria changed the title ~~[WIP][NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread)~~ [NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread) Aug 22, 2018

Sameesunkaria removed the 🚧 wip label Aug 22, 2018

Cells no longer flash on reload

2bf6005

Sameesunkaria and others added 2 commits August 22, 2018 21:02

Add images to swipe actions

8c553dd

Fix a bug that was preventing a section header to be laid out

4d269db

filipealva reviewed Aug 23, 2018

View reviewed changes

Use top constraint to center SubscriptionSectionView label's since so…

5653a2b

…metimes we were not being able to get the correct center y in runtime

Sameesunkaria reviewed Aug 23, 2018

View reviewed changes

filipealva approved these changes Aug 23, 2018

View reviewed changes

cardoso suggested changes Aug 24, 2018

View reviewed changes

Sameesunkaria added 10 commits August 27, 2018 18:17

Move subscription action methods to struct SubscriptionsClient

54e9ea0

Fix glitch where the swipe actions would close intermittently

9ded17d

Exempt swipe actions from getting themed

a4552e6

Automatically close swipe actions when favorite is tapped

4963728

Fix automatically closing swipe actions glitch at launch

221e97f

Merge branch 'develop' into feature/implement_actions_subscriptions.1518

ea6829c

Rename reloadData to updateVisibleCells

acb2baf

Update DifferenceKit to v0.5.2

2a981ae

Merge branch 'develop' into feature/implement_actions_subscriptions.1518

87c1219

Fix merge conflicts

40f9764

rafaelks commented Aug 29, 2018

View reviewed changes

Add pt-BR strings

11141b6

rafaelks merged commit 887730f into develop Aug 30, 2018

rafaelks deleted the feature/implement_actions_subscriptions.1518 branch August 30, 2018 00:08

[NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread) #1960

[NEW] Add the actions to the Subscription (Hide, Leave, Mark as Read & Unread) #1960

Uh oh!

Conversation

rafaelks commented Jul 13, 2018 • edited by Sameesunkaria Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's new in this PR

Uh oh!

codecov bot commented Jul 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

filipealva commented Aug 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipealva commented Aug 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipealva left a comment

Choose a reason for hiding this comment

Uh oh!

cardoso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafaelks commented Aug 29, 2018

Uh oh!

Sameesunkaria commented Aug 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rafaelks commented Jul 13, 2018 •

edited by Sameesunkaria

Loading

codecov bot commented Jul 27, 2018 •

edited

Loading