Skip to content
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

Jump to bottom

Remove VerticalStackLayout from StateContainerController #1262

Merged
merged 9 commits into from
Jun 23, 2023
Merged

Remove VerticalStackLayout from StateContainerController #1262

merged 9 commits into from
Jun 23, 2023

Conversation

TheCodeTraveler
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler commented Jun 21, 2023 •
edited
Loading

Description of Change

This PR removes the VerticalStackLayout from the StateContainer.

.NET MAUI has different behavior than Xamarin.Forms for StackLayout. In .NET MAUI, VerticalStackLayout expands to the size of its children. When the children require more space than is available on the screen, VerticalStackLayout will continue to "display" the additional View below the visible area on the screen.

The example below uses a ScrollView inside of StateContainer. When StateContainerController places the ScrollView inside of a VerticalStackLayout, it does not scroll because the size of the VerticalStackLayout extends past the bottom of the screen.

StateContainer with VerticalStackLayout StateContainer without VerticalStackLayout (Fixed)
Cannot scroll the ScrollView because VerticalStackLayout has laid out its children past the bottom of the screen (notice how the Yellow color extends past the Button with a BackgroundColor = Colors.Pink) Can scroll because the ScrollView is properly sized
Simulator Screen Recording - iPhone 14 Pro - 2023-06-21 at 18 32 41 Simulator Screen Recording - iPhone 14 Pro - 2023-06-21 at 18 31 28

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

Additional information

I've also updated StateContainerController to use IList<IView> originalContent instead of List<View> originalContent to avoid the need to cast item when adding it to the list.

@TheCodeTraveler TheCodeTraveler merged commit 76a5c8c into main Jun 23, 2023
@TheCodeTraveler TheCodeTraveler deleted the Remove-`VerticalStackLayout`-from-`StateContainer` branch June 23, 2023 14:48
@TheCodeTraveler TheCodeTraveler mentioned this pull request Jun 23, 2023
Open
2 tasks
@BobSammers
Copy link

Sorry I'm late to question this commit, but is this the correct way to treat this problem? The VerticalStackLayout had to go, obviously (I found myself here because of issues it is causing in my layout), but I'm concerned about maintaining the principle that the switched view should fill a containing Grid. Why is this desirable? Am I missing something?

What if the consumer wants the View to only go in a particular cell? Why force it to fill the containing Grid? It seems to me the correct behaviour is to respect Grid.Row, Grid.Column, Grid.RowSpan, etc., either on the <toolkit:StateContainer.StateViews>, or on the root of each of the views inside this group (which allows variations upon switching).

If the current behaviour was kept unless these properties were set, that should pretty much eliminate concerns over breaking changes.

Wrapping the View selector in a single-celled Grid is a partial workaround, but it introduces friction, is not intuitive and doesn't allow different views to occupy different locations in the containing Grid's cell structure.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pictos pictos pictos approved these changes

@VladislavAntonyuk VladislavAntonyuk VladislavAntonyuk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] StateContainer - Constraints are not working properly
4 participants
@TheCodeTraveler @BobSammers @pictos @VladislavAntonyuk