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

Do not show user indicators when the view controller is not visible #5804

Merged
merged 3 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,23 @@ import CommonKit
self.presenter = presenter
}

@objc func presentActivityIndicator() {
presentActivityIndicator(label: VectorL10n.homeSyncing)
@objc func presentLoadingIndicator() {
presentLoadingIndicator(label: VectorL10n.homeSyncing)
}

@objc func presentActivityIndicator(label: String) {
@objc func presentLoadingIndicator(label: String) {
guard loadingIndicator == nil else {
// The app is very liberal with calling `presentActivityIndicator` (often not matched by corresponding `removeCurrentActivityIndicator`),
// The app is very liberal with calling `presentLoadingIndicator` (often not matched by corresponding `dismissLoadingIndicator`),
// so there is no reason to keep adding new indiciators if there is one already showing.
return
}

MXLog.debug("[UserIndicatorPresenterWrapper] Present loading indicator")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MXLog.debug("[UserIndicatorPresenterWrapper] Present loading indicator")
MXLog.debug("[UserIndicatorPresenterWrapper] Present activity indicator")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think activity does not belong here, because that name is currently only associated with UIActivityIndicator, which this system is not using. If anything I can probably rename the presentActivityIndicator methods.

loadingIndicator = presenter.present(.loading(label: label, isInteractionBlocking: false))
}

@objc func dismissActivityIndicator() {
@objc func dismissLoadingIndicator() {
MXLog.debug("[UserIndicatorPresenterWrapper] Dismiss loading indicator")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MXLog.debug("[UserIndicatorPresenterWrapper] Dismiss loading indicator")
MXLog.debug("[UserIndicatorPresenterWrapper] Dismiss activity indicator")

loadingIndicator = nil
}

Expand Down
15 changes: 10 additions & 5 deletions Riot/Modules/Common/Recents/RecentsViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ @interface RecentsViewController () <CreateRoomCoordinatorBridgePresenterDelegat
// when the user selects it.
UISearchBar *tableSearchBar;

// Flag indicating whether the view controller is (at least partially) visible and not dissapearing
BOOL isViewVisible;

// Observe kThemeServiceDidChangeThemeNotification to handle user interface theme change.
__weak id kThemeServiceDidChangeThemeNotificationObserver;
}
Expand Down Expand Up @@ -264,6 +267,7 @@ - (void)didReceiveMemoryWarning
- (void)viewWillAppear:(BOOL)animated
{
[super viewWillAppear:animated];
isViewVisible = YES;

[self.screenTracker trackScreen];

Expand Down Expand Up @@ -301,6 +305,7 @@ - (void)viewWillAppear:(BOOL)animated
- (void)viewWillDisappear:(BOOL)animated
{
[super viewWillDisappear:animated];
isViewVisible = NO;

// Leave potential editing mode
[self cancelEditionMode:NO];
Expand Down Expand Up @@ -2408,24 +2413,24 @@ - (BOOL)providesCustomActivityIndicator {
}

- (void)startActivityIndicatorWithLabel:(NSString *)label {
if (self.indicatorPresenter) {
[self.indicatorPresenter presentActivityIndicatorWithLabel:label];
if (self.indicatorPresenter && isViewVisible) {
[self.indicatorPresenter presentLoadingIndicatorWithLabel:label];
} else {
[super startActivityIndicator];
}
}

- (void)startActivityIndicator {
if (self.indicatorPresenter) {
[self.indicatorPresenter presentActivityIndicator];
if (self.indicatorPresenter && isViewVisible) {
[self.indicatorPresenter presentLoadingIndicator];
} else {
[super startActivityIndicator];
}
}

- (void)stopActivityIndicator {
if (self.indicatorPresenter) {
[self.indicatorPresenter dismissActivityIndicator];
[self.indicatorPresenter dismissLoadingIndicator];
} else {
[super stopActivityIndicator];
}
Expand Down
20 changes: 18 additions & 2 deletions Riot/Modules/Room/RoomViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,9 @@ - (void)viewWillDisappear:(BOOL)animated
isAppeared = NO;

[VoiceMessageMediaServiceProvider.sharedProvider pauseAllServices];
[self stopActivityIndicator];

// Stop the loading indicator even if the session is still in progress
[self stopLoadingUserIndicator];
}

- (void)viewDidAppear:(BOOL)animated
Expand Down Expand Up @@ -935,10 +937,14 @@ - (void)onMatrixSessionChange
self.updateRoomReadMarker = NO;
}

#pragma mark - Loading indicators

- (BOOL)providesCustomActivityIndicator {
return [self.delegate roomViewControllerCanDelegateUserIndicators:self];
}

// Override of a legacy method to determine whether to use a newer implementation instead.
// Will be removed in the future https://github.com/vector-im/element-ios/issues/5608
- (void)startActivityIndicator {
if ([self providesCustomActivityIndicator]) {
[self.delegate roomViewControllerDidStartLoading:self];
Expand All @@ -947,6 +953,8 @@ - (void)startActivityIndicator {
}
}

// Override of a legacy method to determine whether to use a newer implementation instead.
// Will be removed in the future https://github.com/vector-im/element-ios/issues/5608
- (void)stopActivityIndicator
{
if (notificationTaskProfile)
Expand All @@ -956,14 +964,22 @@ - (void)stopActivityIndicator
notificationTaskProfile = nil;
}
if ([self providesCustomActivityIndicator]) {
// The legacy super implementation of `stopActivityIndicator` contains a number of checks grouped under `canStopActivityIndicator`
// to determine whether the indicator can be stopped or not (and the method should thus rather be called `stopActivityIndicatorIfPossible`).
// Since the newer indicators are not calling super implementation, the check for `canStopActivityIndicator` has to be performed manually.
if ([self canStopActivityIndicator]) {
[self.delegate roomViewControllerDidStopLoading:self];
[self stopLoadingUserIndicator];
}
} else {
[super stopActivityIndicator];
}
}

- (void)stopLoadingUserIndicator
{
[self.delegate roomViewControllerDidStopLoading:self];
}

- (void)displayRoom:(MXKRoomDataSource *)dataSource
{
// Remove potential preview Data
Expand Down
1 change: 1 addition & 0 deletions changelog.d/5801.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Activity Indicators: Do not show user indicators when the view controller is not visible