Skip to content

[PTECH-34068] Respect shouldAnimateItems in diffing section controllers#138

Open
gmarm wants to merge 2 commits intodevelopfrom
fix/respect-should-animate-in-diffing-controllers
Open

[PTECH-34068] Respect shouldAnimateItems in diffing section controllers#138
gmarm wants to merge 2 commits intodevelopfrom
fix/respect-should-animate-in-diffing-controllers

Conversation

@gmarm
Copy link
Copy Markdown

@gmarm gmarm commented Mar 12, 2026

Ticket: PTECH-34068

Summary

Fixes section controller base classes to respect shouldAnimateItems(from:to:) overrides.

DiffingListSectionController, ManualDiffingListSectionController, and FoundationDiffingListSectionController all override calculateUpdate(from:to:) but never pass shouldAnimateItems(from:to:) to the CollectionViewSectionUpdate initializer, causing it to always default to true.

This means subclasses that override shouldAnimateItems to return false have no effect — updates are always animated

Fix by threading the shouldAnimateItems result through to the shouldAnimate parameter in all three controllers, matching the behavior of the base ListSectionController.calculateUpdate.

`DiffingListSectionController`, `ManualDiffingListSectionController`,
and `FoundationDiffingListSectionController` all override
`calculateUpdate(from:to:)` but do not pass
`shouldAnimateItems(from:to:)` to the `CollectionViewSectionUpdate`
initializer, causing it to always default to `true`.

This means subclasses that override `shouldAnimateItems` to return
`false` have no effect — updates are always animated.

Fix by threading the `shouldAnimateItems` result through to the
`shouldAnimate` parameter in all three controllers, matching the
behavior of the base `ListSectionController.calculateUpdate`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gmarm gmarm requested a review from a team as a code owner March 12, 2026 13:13
@gmarm gmarm changed the title Respect shouldAnimateItems in diffing section controllers [PTECH-34068] Respect shouldAnimateItems in diffing section controllers Mar 12, 2026
Dimentar added a commit that referenced this pull request Apr 7, 2026
https://traderepublic.atlassian.net/browse/PTECH-34241

## Context

CI is broken on all PRs because `macOS-latest` now maps to **macOS 15**
(since Sep 2025), which only ships with **Xcode 16.x**. The workflow
still references `Xcode_15.4.app`, which doesn't exist on the runner:

```
xcrun: error: missing DEVELOPER_DIR path: /Applications/Xcode_15.4.app/Contents/Developer
```

## Changes

### Workflow (`.github/workflows/unittests.yml`)
- **Pin runner to `macos-15`** instead of `macOS-latest` to prevent
future breakage when GitHub rolls the label forward
- **Update `DEVELOPER_DIR` to `Xcode_16.4.app`** — the default and most
stable Xcode on the macOS 15 image
- **Bump `actions/checkout` from v2 to v4** — v2 uses a deprecated
Node.js runtime

### Code signing (`fastlane/Fastfile`)
- **Disable code signing in CI test builds** — Xcode 16 enforces
stricter signing validation, causing `No signing certificate "Mac
Development" found` even for simulator-only test runs. Added `xcargs`
overrides (`CODE_SIGN_IDENTITY`, `CODE_SIGNING_REQUIRED`,
`CODE_SIGNING_ALLOWED`) to both `scan` calls

### Test fix (`SectionKit/Tests/`)
- **Fix `UICollectionViewApplyTests` for iOS 18+** — `UIWindow()`
without a `UIWindowScene` no longer attaches views to the window
hierarchy, so `collectionView.window` returns `nil` and batch update
tests take the wrong code path. Added overridable `_window` property to
`MockCollectionView` instead of relying on real window attachment

### CODEOWNERS
- Updated ownership to `@traderepublic/atom-mobile-platform-ios`

## Acceptance Criteria

- [ ] CI passes on this PR (Carthage bootstrap + unit tests)
- [ ] After merge, re-triggered CI on PR #138 also passes
- [x] Apply Sonar -
https://traderepublic.atlassian.net/wiki/spaces/Platform/pages/3637739558/Set+Up+Sonar+Community#Add-a-GitHub-Project

## Testing Steps

1. Verify the CI workflow runs successfully on this PR
2. After merge, retrigger CI on an existing open PR (e.g. #138) to
confirm it picks up the fix

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@gmarm gmarm requested a review from a team as a code owner April 8, 2026 06:38
@gmarm gmarm enabled auto-merge April 8, 2026 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants