Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Remove unnecessary edge inset for vertically scrolling groups; Fixes … #13380

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

hartez
Copy link
Contributor

@hartez hartez commented Jan 12, 2021

Description of Change

The UIEdgeInsets adjustment to make spacing between group headers and the group itself was adding the horizontal item spacing value to the left edge of vertically scrolling groups. These changes remove that unnecessary extra space (which, in addition to making the left edge of the group offset incorrectly, also would cause the last column of a grid layout to move to the next row).

Issues Resolved

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Control Gallery -> CollectionView Gallery -> Grouping Galleries -> Grouping, Grid

There should be two columns of items in each group; if there's just one, this is still broken.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@hartez hartez added this to the 5.0.0 milestone Jan 12, 2021
@hartez hartez added this to To do in vNext+1 (5.0.0) via automation Jan 12, 2021
@hartez hartez moved this from To do to In Review in vNext+1 (5.0.0) Jan 12, 2021
@PureWeen
Copy link
Contributor

@hartez when you scroll the CollectionView the sum part at the bottom fluctuates between one to two columns. New Issue? Or should that be fixed as part of this issue?

image

@samhouts samhouts added ControlGallery i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often t/bug 🐛 labels Jan 15, 2021
Copy link
Contributor

@rachelkang rachelkang left a comment

Choose a reason for hiding this comment

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

seeing the exact same behavior (2 columns instead of 1), including the bug @PureWeen described on 4.8.0 branch generally

Simulator Screen Shot - iPod touch (7th generation) - 2021-01-15 at 11 49 38

@PureWeen PureWeen removed the request for review from mattleibow January 15, 2021 18:28
@hartez
Copy link
Contributor Author

hartez commented Jan 19, 2021

@PureWeen @rachelkang Yes, this should be addressed in another PR.

I created an issue for it: #13464

@rachelkang
Copy link
Contributor

@hartez is there a project / test file we can test against? The testing procedure detailed passes fine without this fix as well

@hartez
Copy link
Contributor Author

hartez commented Feb 1, 2021

@hartez is there a project / test file we can test against? The testing procedure detailed passes fine without this fix as well

I added HorizontalItemSpacing="5" to the test screen in the PR. The screenshots below are the before (left) and after (right):

screenshots

@hartez hartez added this to To Do in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez moved this from To Do to Needs Review in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez moved this from Needs Review to In Progress in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez removed this from In Progress in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez added this to To Do in 5.0.0 SR 3 via automation Feb 2, 2021
@hartez hartez moved this from To Do to In Progress in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez removed this from In Review in vNext+1 (5.0.0) Feb 2, 2021
@hartez hartez moved this from Issue In Progress to PR Needs Review in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez moved this from PR Needs Review to PR Build/Test Failures in 5.0.0 SR 3 Feb 2, 2021
@hartez
Copy link
Contributor Author

hartez commented Feb 5, 2021

Failing tests are unrelated.

@hartez hartez moved this from PR Build/Test Failures to PR Needs Review in 5.0.0 SR 3 Feb 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/collectionview ControlGallery i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 t/bug 🐛
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Bug] CollectionView with Grouping and GridItemLayout displays wrong number of columns on IOS
7 participants