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

Reduce the number of unnecessary home page reloads #5969

Merged
merged 3 commits into from
Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
46 changes: 26 additions & 20 deletions Riot/Modules/Common/Recents/DataSources/RecentsDataSource.m
Original file line number Diff line number Diff line change
Expand Up @@ -1501,35 +1501,41 @@ - (void)recentsListServiceDidChangeData:(id<RecentsListServiceProtocol>)service
forSection:(RecentsListServiceSection)section
totalCountsChanged:(BOOL)totalCountsChanged
{
NSInteger sectionIndex = -1;
switch (section)
RecentsDataSourceSections *updatedSections = [self makeDataSourceSections];
BOOL hasChangedSections = ![self.sections isEqual:updatedSections];
if (hasChangedSections) {
// If the number or order of sections has changed, we reload all of the data
[self.delegate dataSource:self didCellChange:nil];
return;
}

RecentsDataSourceSectionType sectionType = [self sectionTypeForServiceSection:section];
NSInteger sectionIndex = [self.sections sectionIndexForSectionType:sectionType];
if (sectionIndex >= 0) {
NSIndexPath *indexPath = [NSIndexPath indexPathForRow:0 inSection:(NSUInteger)sectionIndex];
[self.delegate dataSource:self didCellChange:indexPath];
}
}

- (RecentsDataSourceSectionType)sectionTypeForServiceSection:(RecentsListServiceSection)serviceSection
{
switch (serviceSection)
{
case RecentsListServiceSectionInvited:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypeInvites];
break;
return RecentsDataSourceSectionTypeInvites;
case RecentsListServiceSectionFavorited:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypeFavorites];
break;
return RecentsDataSourceSectionTypeFavorites;
case RecentsListServiceSectionPeople:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypePeople];
break;
return RecentsDataSourceSectionTypePeople;
case RecentsListServiceSectionConversation:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypeConversation];
break;
return RecentsDataSourceSectionTypeConversation;
case RecentsListServiceSectionLowPriority:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypeLowPriority];
break;
return RecentsDataSourceSectionTypeLowPriority;
case RecentsListServiceSectionServerNotice:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypeServerNotice];
break;
return RecentsDataSourceSectionTypeServerNotice;
case RecentsListServiceSectionSuggested:
sectionIndex = [self.sections sectionIndexForSectionType:RecentsDataSourceSectionTypeSuggestedRooms];
break;
return RecentsDataSourceSectionTypeSuggestedRooms;
}

RecentsSectionUpdate *update = [[RecentsSectionUpdate alloc] initWithSectionIndex:sectionIndex
totalCountsChanged:totalCountsChanged];
[self.delegate dataSource:self didCellChange:update];
}

#pragma mark - Shrinkable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,11 @@ import Foundation
}
return item.key
}

override func isEqual(_ object: Any?) -> Bool {
guard let other = object as? RecentsDataSourceSections else {
return false
}
return sections == other.sections
}
}
40 changes: 0 additions & 40 deletions Riot/Modules/Common/Recents/Model/RecentsSectionUpdate.swift

This file was deleted.

64 changes: 30 additions & 34 deletions Riot/Modules/Common/Recents/RecentsViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ - (void)viewDidLayoutSubviews

- (void)refreshRecentsTable
{
MXLogDebug(@"[RecentsViewController]: Refreshing recents table view")
// Refresh the tabBar icon badges
[[AppDelegate theDelegate].masterTabBarController refreshTabBarBadges];

Expand Down Expand Up @@ -1031,49 +1032,44 @@ - (void)dataSource:(MXKDataSource *)dataSource didRecognizeAction:(NSString *)ac

- (void)dataSource:(MXKDataSource *)dataSource didCellChange:(id)changes
{
BOOL cellReloaded = NO;
if ([changes isKindOfClass:RecentsSectionUpdate.class])
if ([changes isKindOfClass:NSIndexPath.class])
{
RecentsSectionUpdate *update = (RecentsSectionUpdate*)changes;
if (update.isValid && !update.totalCountsChanged)
NSIndexPath *indexPath = (NSIndexPath *)changes;
UITableViewCell *cell = [self.recentsTableView cellForRowAtIndexPath:indexPath];
if ([cell isKindOfClass:TableViewCellWithCollectionView.class])
{
NSIndexPath *indexPath = [NSIndexPath indexPathForRow:0 inSection:update.sectionIndex];
UITableViewCell *cell = [self.recentsTableView cellForRowAtIndexPath:indexPath];
if ([cell isKindOfClass:TableViewCellWithCollectionView.class])
MXLogDebug(@"[RecentsViewController]: Reloading nested collection view cell in section %ld", indexPath.section);

TableViewCellWithCollectionView *collectionViewCell = (TableViewCellWithCollectionView *)cell;
[collectionViewCell.collectionView reloadData];

CGRect headerFrame = [self.recentsTableView rectForHeaderInSection:indexPath.section];
UIView *headerView = [self.recentsTableView headerViewForSection:indexPath.section];
UIView *updatedHeaderView = [self.dataSource viewForHeaderInSection:indexPath.section withFrame:headerFrame inTableView:self.recentsTableView];
if ([headerView isKindOfClass:SectionHeaderView.class]
&& [updatedHeaderView isKindOfClass:SectionHeaderView.class])
{
TableViewCellWithCollectionView *collectionViewCell = (TableViewCellWithCollectionView *)cell;
[collectionViewCell.collectionView reloadData];
cellReloaded = YES;

CGRect headerFrame = [self.recentsTableView rectForHeaderInSection:update.sectionIndex];
UIView *headerView = [self.recentsTableView headerViewForSection:update.sectionIndex];
UIView *updatedHeaderView = [self.dataSource viewForHeaderInSection:update.sectionIndex withFrame:headerFrame inTableView:self.recentsTableView];
if ([headerView isKindOfClass:SectionHeaderView.class]
&& [updatedHeaderView isKindOfClass:SectionHeaderView.class])
{
SectionHeaderView *sectionHeaderView = (SectionHeaderView *)headerView;
SectionHeaderView *updatedSectionHeaderView = (SectionHeaderView *)updatedHeaderView;
sectionHeaderView.headerLabel = updatedSectionHeaderView.headerLabel;
sectionHeaderView.accessoryView = updatedSectionHeaderView.accessoryView;
sectionHeaderView.rightAccessoryView = updatedSectionHeaderView.rightAccessoryView;
}
SectionHeaderView *sectionHeaderView = (SectionHeaderView *)headerView;
SectionHeaderView *updatedSectionHeaderView = (SectionHeaderView *)updatedHeaderView;
sectionHeaderView.headerLabel = updatedSectionHeaderView.headerLabel;
sectionHeaderView.accessoryView = updatedSectionHeaderView.accessoryView;
sectionHeaderView.rightAccessoryView = updatedSectionHeaderView.rightAccessoryView;
}
} else {
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
} else {
}
else
{

MXLogDebug(@"[RecentsViewController]: Reloading table view section %ld", indexPath.section);
[self.recentsTableView reloadSections:[NSIndexSet indexSetWithIndex:indexPath.section] withRowAnimation:UITableViewRowAnimationNone];
}
} else if (!changes) {
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
} else if (!changes) {
}
else if (!changes)
{

MXLogDebug(@"[RecentsViewController]: Reloading the entire table view");
[self refreshRecentsTable];
}

if (!cellReloaded)
{
[super dataSource:dataSource didCellChange:changes];
}
else
{
// Since we've enabled room list pagination, `refreshRecentsTable` not called in this case.
// Refresh tab bar badges separately.
[[AppDelegate theDelegate].masterTabBarController refreshTabBarBadges];
}
// Since we've enabled room list pagination, `refreshRecentsTable` not called in this case.
// Refresh tab bar badges separately.
[[AppDelegate theDelegate].masterTabBarController refreshTabBarBadges];

[self showEmptyViewIfNeeded];

if (dataSource.state == MXKDataSourceStateReady)
{
[[NSNotificationCenter defaultCenter] postNotificationName:RecentsViewControllerDataReadyNotification
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//
//
// Copyright 2021 New Vector Ltd
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -610,9 +610,10 @@ public class RecentsListService: NSObject, RecentsListServiceProtocol {
multicastDelegate.invoke { $0.recentsListServiceDidChangeData?(self,
forSection: section,
totalCountsChanged: totalCountsChanged) }
} else {
multicastDelegate.invoke { $0.recentsListServiceDidChangeData?(self,
totalCountsChanged: totalCountsChanged) }
}
multicastDelegate.invoke { $0.recentsListServiceDidChangeData?(self,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like we have been doing 2 updates when only one was necessary

totalCountsChanged: totalCountsChanged) }
}

deinit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,35 @@ class RecentsDataSourceSectionsTests: XCTestCase {
]
)
}

func test_equalsIfSameSectionsInSameOrder() {
let original = RecentsDataSourceSections(sectionTypes: [
.favorites,
.invites,
.lowPriority,
.searchedRoom,
])
let sameOrder = RecentsDataSourceSections(sectionTypes: [
.favorites,
.invites,
.lowPriority,
.searchedRoom,
])
let differentOrder = RecentsDataSourceSections(sectionTypes: [
.lowPriority,
.favorites,
.invites,
.searchedRoom,
])
let differentSections = RecentsDataSourceSections(sectionTypes: [
.favorites,
.serverNotice,
.lowPriority,
.searchedRoom,
])

XCTAssertEqual(original, sameOrder)
XCTAssertNotEqual(original, differentOrder)
XCTAssertNotEqual(original, differentSections)
}
}
1 change: 1 addition & 0 deletions changelog.d/5619.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Home: Reduce the number of unnecessary home page reloads