Skip to content

[inflight regression][iOS/macOS] Fix CollectionView, RefreshView, and ScrollView interaction state after IsEnabled toggles - #36628

Merged
kubaflo merged 1 commit into
dotnet:inflight/candidatefrom
SyedAbdulAzeemSF4852:fix-36501-ios
Jul 25, 2026
Merged

[inflight regression][iOS/macOS] Fix CollectionView, RefreshView, and ScrollView interaction state after IsEnabled toggles#36628
kubaflo merged 1 commit into
dotnet:inflight/candidatefrom
SyedAbdulAzeemSF4852:fix-36501-ios

Conversation

@SyedAbdulAzeemSF4852

Copy link
Copy Markdown
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issue Details

Root Cause

  • PR [iOS] Respect InputTransparent when updating UserInteractionEnabled #36305 updated ViewExtensions.UpdateIsEnabled to compute UserInteractionEnabled using both IsEnabled and InputTransparent for non-UIControl views. However, because CollectionView, RefreshView, and ScrollView override MapIsEnabled without invoking the shared implementation, they never adopted this new logic, causing inconsistent UserInteractionEnabled behavior across enable/disable state changes and resulting in failures in CollectionViewScrollsWhenRefreshViewDisabled, ProgressSpinnerWorksWhenReEnabled, VerifyCollectionViewIsEnableState, and ScrollViewInitiallyNotEnabledThenEnabled.

Description of Change

  • Introduced a new UpdateInteractionState extension method in ViewExtensions.cs that centralizes the logic for setting UserInteractionEnabled based on both IsEnabled and InputTransparent, and updated both UpdateIsEnabled and UpdateInputTransparent to use this method.
  • Updated MapIsEnabled methods in handlers for ItemsViewHandler, ItemsViewHandler2, RefreshViewHandler, and ScrollViewHandler to call the base ViewHandler.MapIsEnabled, ensuring interaction state is always recomputed.

Issues Fixed

Fixes #36501

Validated the behaviour in the following platforms

  • Windows
  • Android
  • iOS
  • Mac

Output

TestCase Before After
CollectionViewScrollsWhenRefreshViewDisabled
ScrollViewInitiallyNotEnabledThenEnabled
ProgressSpinnerWorksWhenReEnabled
VerifyCollectionViewIsEnableState

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service dotnet-policy-service Bot added the community ✨ Community Contribution label Jul 17, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey there @@SyedAbdulAzeemSF4852! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jul 17, 2026
@SyedAbdulAzeemSF4852 SyedAbdulAzeemSF4852 changed the title [WIP][inflight regression][iOS/macOS] Fix CollectionView, RefreshView, and ScrollView interaction state after IsEnabled toggles [inflight regression][iOS/macOS] Fix CollectionView, RefreshView, and ScrollView interaction state after IsEnabled toggles Jul 22, 2026
@sheiksyedm
sheiksyedm marked this pull request as ready for review July 24, 2026 11:51
@sheiksyedm
sheiksyedm requested a review from kubaflo July 24, 2026 11:51
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

@sheiksyedm

Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests , maui-pr-devicetests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).

@kubaflo

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Tests Failure Analysis

@kubaflo — test-failure review results are available based on commit 0ae7593.

Maintainers can request a fresh review after new comments, commits, or CI runs by commenting /review tests.

Overall Needs human investigation Failures 190 Regressed vs base 0 Baseline 17 on base

Test Failure Review: Needs human investigation - click to expand

Overall verdict: Needs human investigation. Zero of the 190 distinct failures are provable regressions against the base branch (inflight/candidate, 5 recent builds sampled per pipeline), and 17 also appear on base — but every failure resolved as indeterminate (leg-only base matches, not exact test+platform matches), so none can be dismissed as pre-existing. Combined with 117 build legs that produced no extractable failure, 2 UITest checks that did not finish cleanly, and 3 device-test checks whose clean result could not be confirmed, the failures cannot be safely attributed either way and require a human to classify.

  • i Uncertain — Unexplained / aborted / unverified CI legs (~122 legs): 117 failed build legs yielded no extractable failure (crossgen/NativeAOT/linker/integration-test breaks across Blazor, AOT, RunOniOS, WindowsTemplates), 2 macOS UITest checks did not finish cleanly (cancelled/timed-out), and 3 device-test checks read green but XHarness Failed==0 could not be confirmed.
  • i Uncertain — Android SDK provisioning failures (~16 legs): avdmanager/Failed to find package 'platform-tools;35.0.2' also fails across all 5 sampled base builds but only as a leg match, so it is not dismissible; e.g. to find package 'platform-tools;35.0.2'.
  • i Uncertain — iOS Flyout measurement failures (~96 tests): FlyoutHeaderContentAndFooterAllMeasureCorrectly reports a consistent 34px footer-Y offset (Expected: 761. Actual: 795); this touches the ScrollView/iOS layout area this PR modifies, so it warrants a human look even though it is not a proven base regression.
  • i Uncertain — Shadow-transform NullReferenceExceptions (~11 tests): the Verify*AndShadow family throws NullReferenceException and also fails on base (leg match only), so not dismissible; e.g. VerifyScaleXAndShadow.
  • i Uncertain — Device-test failures & incomplete work items (~8 tests): gesture leak tests, ItemsUpdateWithCollectionChanges, a crashed Helix work item, and GroupedCollectionViewGridLayoutRendersCorrectly (missing baseline snapshot) — all indeterminate.
  • i Uncertain — Build/publish errors (~6 legs): PublishTestResults, DeviceTests*(Windows), and mac UITest result-publish steps exited non-zero with no test-level cause.

Coverage: 175 checks · 105 passing · 70 failing · 0 pending · 0 inaccessible · 2 unmapped · 117 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 3 device-test unverified · 190 unattributed · 0 regressed-vs-base. Deterministic ceiling: Needs human investigation — 2 unmapped checks, 117 unexplained legs, 2 aborted UITest checks, 3 unverified device-test checks, and 190 unattributed failures.

Builds (this PR): maui-pr 1524976, maui-pr-devicetests 1524978, maui-pr-uitests 1524977. Base sampling (inflight/candidate, 5 recent builds per definition): maui-pr 1521712, maui-pr-devicetests 1521714, maui-pr-uitests 1521713.

Recommended action

A maintainer should manually confirm CI health: the base branch (inflight/candidate) is itself red, so re-run the failed legs and confirm whether the ~96 iOS FlyoutHeaderContentAndFooterAllMeasureCorrectly measurement failures are related to this PR's ScrollView/iOS layout changes before merging.

Visual failure comparisons

Full-resolution CI baseline, actual, and diff images are embedded below. They supplement the failure classification and do not change the deterministic verdict ceiling.
Relationship labels use deterministic exact test-and-platform baseline evidence plus exact changed snapshot/test scope; missing or mixed evidence remains Needs human investigation.

Visual comparisons were prepared but could not be published to the asset branch because a Git or API error occurred after image preparation. The deterministic verdict is unaffected; no images are shown for this run.

4 additional visual comparison(s) were omitted by publisher bounds (deduplication, the comparison cap, the discovery/publish time budget, or assets that failed validation).

@kubaflo
kubaflo merged commit fa8ee5f into dotnet:inflight/candidate Jul 25, 2026
105 of 175 checks passed
@github-actions github-actions Bot added this to the .NET 10 SR10 milestone Jul 25, 2026
PureWeen pushed a commit that referenced this pull request Jul 29, 2026
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

### Description of Change

Fixes screenshot publication for the merged `/review tests`
visual-comparison workflow.

New visual assets are now published from an orphan, asset-only
`review-tests-assets-v2` branch. The publisher initializes that branch
with a root commit containing only a marker blob. On every publication
attempt it permits top-level blobs and `pr-<number>` directories, while
rejecting trees at any other path, submodule gitlinks, malformed
entries, and truncated root-tree responses.

The local runner also recognizes report markers only when they occupy a
standalone line, preventing a marker quoted in agent prose from leaking
a junk preamble or duplicate marker into the posted comment.

The legacy `review-tests-assets` branch remains untouched so all
existing `raw.githubusercontent.com` links pinned to its historical
commit SHAs remain reachable.

### Root Cause

Production validation on PRs #36628 and #36404 showed that the merged
workflow activated correctly, gathered evidence, invoked the agent, and
posted the ordinary analysis comments:

- [run
30131857502](https://github.com/dotnet/maui/actions/runs/30131857502) /
[comment on
#36628](#36628 (comment))
- [run
30131857838](https://github.com/dotnet/maui/actions/runs/30131857838) /
[comment on
#36404](#36404 (comment))

Both comments contained zero screenshot panels. Their trusted
pre-activation jobs had `contents: write`, prepared visual evidence, and
successfully created Git blobs, trees, and commits, but failed when
advancing the asset ref:

```text
PATCH repos/dotnet/maui/git/refs/heads/review-tests-assets
HTTP 403: Resource not accessible by integration
```

`review-tests-assets` had originally been initialized by pointing it at
`main`, so every generated asset commit inherited the entire repository
tree, including `.github/workflows`. Updating a ref whose resulting tree
contains workflow files requires GitHub's separate Workflows write
permission, which cannot be granted to `GITHUB_TOKEN`.

### Fix

- Switch new publication to `review-tests-assets-v2`.
- Initialize a missing asset branch as an orphan root containing only
`.review-tests-assets`.
- Never copy the default branch tree into an asset branch.
- Validate existing and concurrently-created branch tips before use.
- Fail closed when the root tree is truncated or contains any top-level
entry that is neither a blob nor a `pr-<positive-number>` directory.
Plain blobs cannot carry `.github/workflows`, so optional files such as
a README remain safe.
- Preserve the existing fast-forward retry behavior for concurrent
publishers.
- Keep the agent job read-only and avoid introducing a broader PAT or
GitHub App credential.
- Extract only standalone report marker or heading lines from local
Copilot output, ignoring quoted markers in explanatory prose.

### Validation

- 171 focused Pester tests pass across gathering, publication, visual
merging, and the local runner, including all 28 publisher tests and
seven new report-extraction regression cases.
- PowerShell parsing passes.
- [Fork Actions smoke run
30153639425](https://github.com/kubaflo/maui/actions/runs/30153639425)
passed using only the job-scoped `GITHUB_TOKEN` with `contents: write`.
It created a zero-parent, marker-only root; advanced
`review-tests-assets-v2` to an asset commit; verified the root contains
only the marker and `pr-36785`; and downloaded the [immutable published
PNG](https://raw.githubusercontent.com/kubaflo/maui/0d9677267ed6521b7530bbd3d4bae929767e4dbb/pr-36785/smoke/run-30153639425-actual.png).
- [Real-evidence fork run
30154060821](https://github.com/kubaflo/maui/actions/runs/30154060821)
replayed the last available `maui-pr-uitests` build for PR #33007
through the unchanged publisher and panel merger. It gathered 98 real
comparisons, published the bounded 24 comparisons as 72
baseline/actual/diff PNGs at asset commit
[`5d114f9`](kubaflo@5d114f9),
rendered 15 screenshot panels within the 45-URL comment limit, and
verified every embedded immutable PNG URL.
- A full local `/review tests` run on PR #33007 gathered all three
available MAUI builds, ran the `claude-opus-4.8` analysis, published 24
comparisons to the upstream orphan asset branch, merged 13 bounded
panels, and [posted the complete
report](#33007 (comment)).
Its `Not ready` verdict matches the deterministic ceiling, and all 39
embedded PNG URLs were verified.
- A full local run on PR #35892 published 22 comparisons at asset commit
[`f50f5a1`](f50f5a1),
merged 12 bounded panels, and [posted the complete
report](#35892 (comment)).
Its `Not ready` verdict matches the deterministic ceiling, and all 36
embedded immutable PNG URLs return HTTP 200.
- A full local run from publisher commit `aad33ce` on PR #36507
published 3 comparisons at asset commit
[`3cb2045`](3cb2045),
merged 3 bounded panels, and [updated the complete
report](#36507 (comment)).
Its `Not ready` verdict matches the deterministic ceiling, all 9
embedded immutable PNG URLs return HTTP 200, and replaying the exact
quoted-marker output through parser commit `30cfb3d` produces one local
marker with no junk preamble.
- Full local runs on PRs
[#36277](#36277 (comment))
and
[#36404](#36404 (comment))
found no publishable visual snapshots and correctly kept the ordinary
analysis unchanged. Their final verdicts match their deterministic
ceilings, covering the no-visual path.
- After these publications, `review-tests-assets-v2` still contains only
`.review-tests-assets` and top-level `pr-<number>` directories; its tip
`3cb2045` fast-forwards from `f67ed0e`.
- `gh aw` v0.82.14 compiles `copilot-review-tests` with 0 errors and 0
warnings; the generated lock and actions lock remain unchanged.
- A focused read-only review found no significant correctness, security,
data-loss, race, or compatibility issues.

### Issues Fixed

Follow-up to #36666.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: kubaflo <kubaflo@users.noreply.github.com>
Copilot-Session: a280b482-e102-4ca0-9ff9-1cfe1946e21f
Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
Copilot-Session: 56ae58e8-a78b-4f24-9920-bc096dfb01fa
kubaflo pushed a commit that referenced this pull request Jul 30, 2026
Replace the PR-specific container workaround with the merged #36628 implementation. Route ScrollView, RefreshView, and both ItemsView handlers through ViewHandler.MapIsEnabled and share interaction-state calculation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 60fb173f-7de4-4b51-822d-b306a63c2bfc
kubaflo pushed a commit that referenced this pull request Jul 30, 2026
Replace the PR-specific container workaround with the merged #36628 implementation. Route ScrollView, RefreshView, and both ItemsView handlers through ViewHandler.MapIsEnabled and share interaction-state calculation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 60fb173f-7de4-4b51-822d-b306a63c2bfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants