Skip to content

[RST-27240] Fixes unexpected cell animation bug #592

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
Aug 25, 2025
Merged

[RST-27240] Fixes unexpected cell animation bug #592

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
a7d7eac
[RST-27240] Reverts 'Revert cell animation fix (#542)'
rpeel-sq Aug 25, 2025
201a59c
Removes xcodes install
rpeel-sq Aug 25, 2025
608ba76
Update XCode version to 16.4
rpeel-sq Aug 25, 2025
7a9b0cc
Renames Install xcodes -> Install aria2
rpeel-sq Aug 25, 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
2 changes: 1 addition & 1 deletion .github/workflows/env.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
xcode_version=16.2
xcode_version=16.4
TUIST_TEST_DEVICE=iPhone SE (3rd generation)
TUIST_TEST_PLATFORM=iOS
4 changes: 2 additions & 2 deletions .github/workflows/run_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ jobs:
- name: Switch to Xcode ${{ env.xcode_version }}
run: sudo xcode-select -s /Applications/Xcode_${{ env.xcode_version }}.app

- name: Install xcodes
run: brew install aria2 xcodesorg/made/xcodes
- name: Install aria2
run: brew install aria2
Comment on lines +49 to +50
Copy link
Member

Choose a reason for hiding this comment

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

I'm going to ask the team for one additional review around the github workflow changes and whether or not we need aria2.

I think aria2 will still be used implicitly in the following step so probably worth keeping around. You might be able to find some hints buried in the build logs but I guess the best way to verify would be to spin up a PR without it and compare the total CI times.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @robmaceachern! This draft PR removed aria2 and CI took 5m 51s. That seems to match previous results, but I'm on board with keeping it around if we'd like to minimize the changes here.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it looks like it's either not being used or makes no real difference. Probably fine to remove!

Copy link
Member

Choose a reason for hiding this comment

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

Oh actually I just noticed this:
image

So it might be that just removing the install step doesn't necessarily mean that aria2 won't already be installed on the machine. Not sure if github includes it or the runner was cached or something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So should I remove or leave 😅?

Copy link
Member

Choose a reason for hiding this comment

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

Haha just leave it 👍🏽


- name: Install iOS ${{ matrix.sdk }}
if: ${{ matrix.installation_required }}
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### Fixed

- Fixed an issue where animations would occur when dequeuing / reusing cells. A layout is now forced without animation before presentation.

### Added

### Removed
Expand Down
39 changes: 0 additions & 39 deletions Development/Sources/Demos/Demo Screens/AnimatedReuseViewController.swift
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion Development/Sources/Demos/Demo Screens/CollectionViewAppearance.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ struct Toggle : Element {

config.apply { toggle in
if toggle.isOn != self.isOn {
toggle.setOn(self.isOn, animated: true)
toggle.setOn(self.isOn, animated: UIView.inheritedAnimationDuration > 0.0)
}
toggle.onToggle = self.onToggle
}
Expand Down
12 changes: 2 additions & 10 deletions Development/Sources/Demos/DemosRootViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ public final class DemosRootViewController : ListViewController
DemoHeader(title: "Other Layouts")
}

Section("testing") { [weak self] in
Section("fuzzing") { [weak self] in

Item(
DemoItem(text: "Fuzz Testing"),
Expand All @@ -368,16 +368,8 @@ public final class DemosRootViewController : ListViewController
self?.push(SupplementaryTestingViewController())
}
)

Item(
DemoItem(text: "Verify Reuse Has No Animation"),
selectionStyle: .selectable(),
onSelect : { _ in
self?.push(AnimatedReuseViewController())
}
)
} header: {
DemoHeader(title: "Testing")
DemoHeader(title: "Fuzz Testing")
}

Section("selection-state") {
Expand Down
14 changes: 14 additions & 0 deletions ListableUI/Sources/ListView/ListView.Delegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ extension ListView
item.willDisplay(cell: cell, in: collectionView, for: indexPath)

self.displayedItems[ObjectIdentifier(cell)] = item

UIView.performWithoutAnimation {
/// Force a layout of the cell before it is displayed, so that any implicit animations
/// are avoided. This ensures that cases like toggling a switch on and off are
/// not animated as the cell comes into view.
cell.layoutIfNeeded()
}
}

func collectionView(
Expand Down Expand Up @@ -182,6 +189,13 @@ extension ListView
headerFooter.collectionViewWillDisplay(view: container)

self.displayedSupplementaryItems[ObjectIdentifier(container)] = headerFooter

UIView.performWithoutAnimation {
/// Force a layout of the cell before it is displayed, so that any implicit animations
/// are avoided. This ensures that cases like toggling a switch on and off are
/// not animated as the cell comes into view.
container.layoutIfNeeded()
}
}

func collectionView(
Expand Down
Loading