Skip to content

Added a hack to work around a cell reuse height calculation bug #634

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 27, 2025
Merged

Added a hack to work around a cell reuse height calculation bug #634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
583e62b
Added a hack to work around a cell reuse height calculation bug
mpretty-cyro Nov 18, 2025
6171529
Tweak the build script to fix a parsing error
mpretty-cyro Nov 18, 2025
a2dabef
Made code coverage output smaller, fixed broken unit tests
mpretty-cyro Nov 18, 2025
8ff3a7d
Fix issue found during QA
mpretty-cyro Nov 24, 2025
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
8 changes: 5 additions & 3 deletions Scripts/build_ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ if [[ "$MODE" == "test" ]]; then
echo "----------------------------------------------------"
echo "Checking for test failures in xcresult bundle..."

xcresultparser --output-format cli --no-test-result --coverage ./build/artifacts/testResults.xcresult
xcresultparser --output-format cli --no-test-result --coverage-report-format targets --coverage ./build/artifacts/testResults.xcresult
parser_output=$(xcresultparser --output-format cli --no-test-result ./build/artifacts/testResults.xcresult)

build_errors_count=$(echo "$parser_output" | grep "Number of errors" | awk '{print $NF}' | grep -o '[0-9]*' || echo "0")
failed_tests_count=$(echo "$parser_output" | grep "Number of failed tests" | awk '{print $NF}' | grep -o '[0-9]*' || echo "0")
# Strip ANSI color codes before parsing
clean_parser_output=$(echo "$parser_output" | sed 's/\[[0-9;]*m//g')
build_errors_count=$(echo "$clean_parser_output" | grep "Number of errors" | awk '{print $NF}' | grep -o '[0-9]*' || echo "0")
failed_tests_count=$(echo "$clean_parser_output" | grep "Number of failed tests" | awk '{print $NF}' | grep -o '[0-9]*' || echo "0")

if [ "${build_errors_count:-0}" -gt 0 ] || [ "${failed_tests_count:-0}" -gt 0 ]; then
echo ""
Expand Down
16 changes: 8 additions & 8 deletions Session.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8420,7 +8420,7 @@
CODE_SIGN_IDENTITY = "iPhone Developer";
COMPILE_LIB_SESSION = "";
COPY_PHASE_STRIP = NO;
CURRENT_PROJECT_VERSION = 658;
CURRENT_PROJECT_VERSION = 660;
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
ENABLE_TESTABILITY = YES;
Expand Down Expand Up @@ -8460,7 +8460,7 @@
IPHONEOS_DEPLOYMENT_TARGET = 15.6;
LIB_SESSION_SOURCE_DIR = "${SRCROOT}/../LibSession-Util";
LOCALIZED_STRING_SWIFTUI_SUPPORT = NO;
MARKETING_VERSION = 2.14.6;
MARKETING_VERSION = 2.14.7;
ONLY_ACTIVE_ARCH = YES;
OTHER_CFLAGS = "-Werror=protocol";
OTHER_SWIFT_FLAGS = "-D DEBUG -Xfrontend -warn-long-expression-type-checking=100";
Expand Down Expand Up @@ -8501,7 +8501,7 @@
CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
CODE_SIGN_IDENTITY = "iPhone Distribution";
COMPILE_LIB_SESSION = "";
CURRENT_PROJECT_VERSION = 658;
CURRENT_PROJECT_VERSION = 660;
ENABLE_BITCODE = NO;
ENABLE_MODULE_VERIFIER = YES;
ENABLE_STRICT_OBJC_MSGSEND = YES;
Expand Down Expand Up @@ -8536,7 +8536,7 @@
IPHONEOS_DEPLOYMENT_TARGET = 15.6;
LIB_SESSION_SOURCE_DIR = "${SRCROOT}/../LibSession-Util";
LOCALIZED_STRING_SWIFTUI_SUPPORT = NO;
MARKETING_VERSION = 2.14.6;
MARKETING_VERSION = 2.14.7;
ONLY_ACTIVE_ARCH = NO;
OTHER_CFLAGS = (
"-DNS_BLOCK_ASSERTIONS=1",
Expand Down Expand Up @@ -8987,7 +8987,7 @@
CODE_SIGN_IDENTITY = "iPhone Developer";
COMPILE_LIB_SESSION = YES;
COPY_PHASE_STRIP = NO;
CURRENT_PROJECT_VERSION = 658;
CURRENT_PROJECT_VERSION = 660;
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
ENABLE_TESTABILITY = YES;
Expand Down Expand Up @@ -9026,7 +9026,7 @@
IPHONEOS_DEPLOYMENT_TARGET = 15.6;
LIB_SESSION_SOURCE_DIR = "${SRCROOT}/../LibSession-Util";
LOCALIZED_STRING_SWIFTUI_SUPPORT = NO;
MARKETING_VERSION = 2.14.6;
MARKETING_VERSION = 2.14.7;
ONLY_ACTIVE_ARCH = YES;
OTHER_CFLAGS = (
"-fobjc-arc-exceptions",
Expand Down Expand Up @@ -9577,7 +9577,7 @@
CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
CODE_SIGN_IDENTITY = "iPhone Distribution";
COMPILE_LIB_SESSION = YES;
CURRENT_PROJECT_VERSION = 658;
CURRENT_PROJECT_VERSION = 660;
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
GCC_NO_COMMON_BLOCKS = YES;
Expand Down Expand Up @@ -9610,7 +9610,7 @@
IPHONEOS_DEPLOYMENT_TARGET = 15.6;
LIB_SESSION_SOURCE_DIR = "${SRCROOT}/../LibSession-Util";
LOCALIZED_STRING_SWIFTUI_SUPPORT = NO;
MARKETING_VERSION = 2.14.6;
MARKETING_VERSION = 2.14.7;
ONLY_ACTIVE_ARCH = NO;
OTHER_CFLAGS = (
"-DNS_BLOCK_ASSERTIONS=1",
Expand Down
1 change: 1 addition & 0 deletions Session/Home/Message Requests/MessageRequestsViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ class MessageRequestsViewModel: SessionTableViewModel, NavigatableStateHolder, O
threadCanWrite: false, // Irrelevant for the MessageRequestsViewModel
threadCanUpload: false // Irrelevant for the MessageRequestsViewModel
),
canReuseCell: true,
accessibility: Accessibility(
identifier: "Message request"
),
Expand Down
1 change: 1 addition & 0 deletions Session/Media Viewing & Editing/PhotoCollectionPickerViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class PhotoCollectionPickerViewModel: SessionTableViewModel, ObservableTableSour

return SessionCell.Info(
id: TableItem(collection: collection),
canReuseCell: true,
leadingAccessory: .iconAsync(
size: .extraLarge,
source: lastAssetItem?.source,
Expand Down
1 change: 1 addition & 0 deletions Session/Settings/BlockedContactsViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ public class BlockedContactsViewModel: SessionTableViewModel, NavigatableStateHo
.map { model -> SessionCell.Info<TableItem> in
SessionCell.Info(
id: model,
canReuseCell: true,
leadingAccessory: .profile(id: model.id, profile: model.profile),
title: (
model.profile?.displayName() ??
Expand Down
1 change: 1 addition & 0 deletions Session/Settings/NotificationContentViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class NotificationContentViewModel: SessionTableViewModel, NavigatableStateHolde
.map { previewType in
SessionCell.Info(
id: previewType,
canReuseCell: true,
title: previewType.name,
trailingAccessory: .radio(
isSelected: (state.previewType == previewType)
Expand Down
1 change: 1 addition & 0 deletions Session/Settings/NotificationSoundViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class NotificationSoundViewModel: SessionTableViewModel, NavigationItemSource, N
.map { sound in
SessionCell.Info(
id: sound,
canReuseCell: true,
title: {
guard sound != .note else {
return "\(sound.displayName) (default)"
Expand Down
1 change: 1 addition & 0 deletions Session/Shared/SessionListViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ class SessionListViewModel<T: Listable>: SessionTableViewModel, NavigationItemSo
.map { option in
SessionCell.Info(
id: option,
canReuseCell: true,
title: option.title,
subtitle: option.subtitle,
trailingAccessory: .radio(
Expand Down
22 changes: 21 additions & 1 deletion Session/Shared/SessionTableViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ class SessionTableViewController<ViewModel>: BaseVC, UITableViewDataSource, UITa
result.sectionHeaderTopPadding = 0
result.rowHeight = UITableView.automaticDimension
result.estimatedRowHeight = UITableView.automaticDimension

// FIXME: Refactor this screen to SwiftUI and avoid using this hack
/// There are a bunch of cells which dynamically calculate their heights and when they get reused by other cells the height can
/// incorrectly remain, in order to avoid this we register a bunch of cells with generic identifiers so we can avoid reusing cells in
/// these cases (these screens generally don't have a lot of cells so it shouldn't be an issue)
(0..<50).forEach { index1 in
(0..<50).forEach { index2 in
result.register(SessionCell.self, forCellReuseIdentifier: "\(index1)-\(index2)")
}
}

return result
}()
Expand Down Expand Up @@ -443,7 +453,17 @@ class SessionTableViewController<ViewModel>: BaseVC, UITableViewDataSource, UITa
func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
let section: SectionModel = tableData[indexPath.section]
let info: SessionCell.Info<TableItem> = section.elements[indexPath.row]
let cell: UITableViewCell = tableView.dequeue(type: viewModel.cellType.viewType.self, for: indexPath)
let cell: UITableViewCell

// FIXME: Refactor this screen to SwiftUI and avoid using this hack
/// There are a bunch of cells which dynamically calculate their heights and when they get reused by other cells the height can
/// incorrectly remain, in order to avoid this we register a bunch of cells with generic identifiers so we can avoid reusing cells in
/// these cases (these screens generally don't have a lot of cells so it shouldn't be an issue)
switch (viewModel.cellType.viewType.self, info.canReuseCell) {
case (is SessionCell.Type, false):
cell = tableView.dequeueReusableCell(withIdentifier: "\(indexPath.section)-\(indexPath.row)", for: indexPath)
default: cell = tableView.dequeue(type: viewModel.cellType.viewType.self, for: indexPath)
}

switch (cell, info) {
case (let cell as SessionCell, _):
Expand Down
16 changes: 16 additions & 0 deletions Session/Shared/Types/SessionCell+Info.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import SessionMessagingKit
extension SessionCell {
public struct Info<ID: Hashable & Differentiable>: Equatable, Hashable, Differentiable {
let id: ID
let canReuseCell: Bool
let position: Position
let leadingAccessory: SessionCell.Accessory?
let title: TextInfo?
Expand All @@ -32,6 +33,7 @@ extension SessionCell {

init(
id: ID,
canReuseCell: Bool = false, // FIXME: This shouldn't be needed but is a hack to prevent layout bugs on cell reuse
position: Position = .individual,
leadingAccessory: SessionCell.Accessory? = nil,
title: SessionCell.TextInfo? = nil,
Expand All @@ -46,6 +48,7 @@ extension SessionCell {
onTapView: (@MainActor (UIView?) -> Void)? = nil
) {
self.id = id
self.canReuseCell = canReuseCell
self.position = position
self.leadingAccessory = leadingAccessory
self.title = title
Expand All @@ -66,6 +69,7 @@ extension SessionCell {

public func hash(into hasher: inout Hasher) {
id.hash(into: &hasher)
canReuseCell.hash(into: &hasher)
position.hash(into: &hasher)
leadingAccessory.hash(into: &hasher)
title.hash(into: &hasher)
Expand All @@ -80,6 +84,7 @@ extension SessionCell {
public static func == (lhs: Info<ID>, rhs: Info<ID>) -> Bool {
return (
lhs.id == rhs.id &&
lhs.canReuseCell == rhs.canReuseCell &&
lhs.position == rhs.position &&
lhs.leadingAccessory == rhs.leadingAccessory &&
lhs.title == rhs.title &&
Expand All @@ -96,6 +101,7 @@ extension SessionCell {
public func updatedPosition(for index: Int, count: Int) -> Info {
return Info(
id: id,
canReuseCell: canReuseCell,
position: Position.with(index, count: count),
leadingAccessory: leadingAccessory,
title: title,
Expand All @@ -120,6 +126,7 @@ public extension SessionCell.Info {

init(
id: ID,
canReuseCell: Bool = false,
position: Position = .individual,
accessory: SessionCell.Accessory,
styling: SessionCell.StyleInfo = SessionCell.StyleInfo(),
Expand All @@ -129,6 +136,7 @@ public extension SessionCell.Info {
onTap: (@MainActor () -> Void)? = nil
) {
self.id = id
self.canReuseCell = canReuseCell
self.position = position
self.leadingAccessory = accessory
self.title = nil
Expand All @@ -147,6 +155,7 @@ public extension SessionCell.Info {

init(
id: ID,
canReuseCell: Bool = false,
position: Position = .individual,
leadingAccessory: SessionCell.Accessory,
trailingAccessory: SessionCell.Accessory,
Expand All @@ -156,6 +165,7 @@ public extension SessionCell.Info {
confirmationInfo: ConfirmationModal.Info? = nil
) {
self.id = id
self.canReuseCell = canReuseCell
self.position = position
self.leadingAccessory = leadingAccessory
self.title = nil
Expand All @@ -174,6 +184,7 @@ public extension SessionCell.Info {

init(
id: ID,
canReuseCell: Bool = false,
position: Position = .individual,
leadingAccessory: SessionCell.Accessory? = nil,
title: String,
Expand All @@ -185,6 +196,7 @@ public extension SessionCell.Info {
onTap: (@MainActor () -> Void)? = nil
) {
self.id = id
self.canReuseCell = canReuseCell
self.position = position
self.leadingAccessory = leadingAccessory
self.title = SessionCell.TextInfo(title, font: .title)
Expand All @@ -203,6 +215,7 @@ public extension SessionCell.Info {

init(
id: ID,
canReuseCell: Bool = false,
position: Position = .individual,
leadingAccessory: SessionCell.Accessory? = nil,
title: SessionCell.TextInfo,
Expand All @@ -214,6 +227,7 @@ public extension SessionCell.Info {
onTap: (@MainActor () -> Void)? = nil
) {
self.id = id
self.canReuseCell = canReuseCell
self.position = position
self.leadingAccessory = leadingAccessory
self.title = title
Expand All @@ -232,6 +246,7 @@ public extension SessionCell.Info {

init(
id: ID,
canReuseCell: Bool = false,
position: Position = .individual,
leadingAccessory: SessionCell.Accessory? = nil,
title: String,
Expand All @@ -245,6 +260,7 @@ public extension SessionCell.Info {
onTapView: (@MainActor (UIView?) -> Void)? = nil
) {
self.id = id
self.canReuseCell = canReuseCell
self.position = position
self.leadingAccessory = leadingAccessory
self.title = SessionCell.TextInfo(title, font: .title)
Expand Down
1 change: 1 addition & 0 deletions Session/Shared/UserListViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ class UserListViewModel<T: ProfileAssociated & FetchableRecord>: SessionTableVie

return SessionCell.Info(
id: .user(userInfo.profileId),
canReuseCell: true,
leadingAccessory: .profile(
id: userInfo.profileId,
profile: userInfo.profile,
Expand Down
10 changes: 10 additions & 0 deletions Session/Shared/Views/SessionCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,14 @@ public class SessionCell: UITableViewCell {
titleLabel.text = ""
titleLabel.themeTextColor = .textPrimary
titleLabel.alpha = 1
titleLabel.preferredMaxLayoutWidth = 0
subtitleLabel.isUserInteractionEnabled = false
subtitleLabel.attributedText = nil
subtitleLabel.themeTextColor = .textPrimary
subtitleLabel.preferredMaxLayoutWidth = 0
expandableDescriptionLabel.themeAttributedText = nil
expandableDescriptionLabel.themeTextColor = .textPrimary
expandableDescriptionLabel.preferredMaxLayoutWidth = 0
trailingAccessoryView.prepareForReuse()
trailingAccessoryView.alpha = 1
trailingAccessoryFillConstraint.isActive = false
Expand Down Expand Up @@ -521,6 +524,7 @@ public class SessionCell: UITableViewCell {
}

// Content
let oldTitle: String? = titleLabel.text
let contentStackViewHorizontalInset: CGFloat = (
(backgroundLeftConstraint.constant + (-backgroundRightConstraint.constant)) +
(contentStackViewLeadingConstraint.constant + (-contentStackViewTrailingConstraint.constant))
Expand Down Expand Up @@ -570,6 +574,12 @@ public class SessionCell: UITableViewCell {
maxContentWidth: (tableSize.width - contentStackViewHorizontalInset),
using: dependencies
)

/// Need to force a re-layout if the title changes as it might not size the content correctly if we don't
if titleLabel.text != oldTitle {
titleLabel.setNeedsLayout()
titleLabel.layoutIfNeeded()
}
}

// MARK: - Interaction
Expand Down
6 changes: 6 additions & 0 deletions SessionTests/Settings/NotificationContentViewModelSpec.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class NotificationContentViewModelSpec: AsyncSpec {
equal([
SessionCell.Info(
id: Preferences.NotificationPreviewType.nameAndPreview,
canReuseCell: true,
position: .top,
title: "notificationsContentShowNameAndContent".localized(),
trailingAccessory: .radio(
Expand All @@ -82,6 +83,7 @@ class NotificationContentViewModelSpec: AsyncSpec {
),
SessionCell.Info(
id: Preferences.NotificationPreviewType.nameNoPreview,
canReuseCell: true,
position: .middle,
title: "notificationsContentShowNameOnly".localized(),
trailingAccessory: .radio(
Expand All @@ -90,6 +92,7 @@ class NotificationContentViewModelSpec: AsyncSpec {
),
SessionCell.Info(
id: Preferences.NotificationPreviewType.noNameNoPreview,
canReuseCell: true,
position: .bottom,
title: "notificationsContentShowNoNameOrContent".localized(),
trailingAccessory: .radio(
Expand Down Expand Up @@ -118,6 +121,7 @@ class NotificationContentViewModelSpec: AsyncSpec {
equal([
SessionCell.Info(
id: Preferences.NotificationPreviewType.nameAndPreview,
canReuseCell: true,
position: .top,
title: "notificationsContentShowNameAndContent".localized(),
trailingAccessory: .radio(
Expand All @@ -126,6 +130,7 @@ class NotificationContentViewModelSpec: AsyncSpec {
),
SessionCell.Info(
id: Preferences.NotificationPreviewType.nameNoPreview,
canReuseCell: true,
position: .middle,
title: "notificationsContentShowNameOnly".localized(),
trailingAccessory: .radio(
Expand All @@ -134,6 +139,7 @@ class NotificationContentViewModelSpec: AsyncSpec {
),
SessionCell.Info(
id: Preferences.NotificationPreviewType.noNameNoPreview,
canReuseCell: true,
position: .bottom,
title: "notificationsContentShowNoNameOrContent".localized(),
trailingAccessory: .radio(
Expand Down