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

ContentSizer sets Width to 0 if layout hasn't completed #85

Closed
michael-hawker opened this issue Jun 12, 2023 · 2 comments · Fixed by #86
Closed

ContentSizer sets Width to 0 if layout hasn't completed #85

michael-hawker opened this issue Jun 12, 2023 · 2 comments · Fixed by #86
Labels
bug Something isn't working

Comments

@michael-hawker
Copy link
Member

Related to #26

Discovered as part of work on DataTable experiment in labs in this branch/commit here: CommunityToolkit/Labs-Windows@47f46df#diff-b874aea5366fbeacbd08eec0606a72bf3b458be31ff250be121b9c9f5ec7b49d

Basically, when the target control was being set in this scenario, the layout hadn't occurred yet on the, so the DesiredSize.Width is still '0'; which means the Width of the component is set to 0 and is never updated or changed making it invisible.

Instead, if the DesiredSize is not set yet, then we shouldn't manipulate the Width of the control, as the patch above did.

We set the Width property is that's modified by the sizing behavior:

Windows/components/Sizers/src/ContentSizer/ContentSizer.Events.cs

Lines 36 to 50 in 744e6ab

protected override bool OnDragHorizontal(double horizontalChange)
{
if (TargetControl == null)
{
return true;
}
horizontalChange = IsDragInverted ? -horizontalChange : horizontalChange;
if (!IsValidWidth(TargetControl, _currentSize + horizontalChange, ActualWidth))
{
return false;
}
TargetControl.Width = _currentSize + horizontalChange;

However, when we start to drag we measure the ActualWidth (or Height) here:

Windows/components/Sizers/src/ContentSizer/ContentSizer.Events.cs

Lines 24 to 33 in 744e6ab

protected override void OnDragStarting()
{
if (TargetControl != null)
{
_currentSize =
Orientation == Orientation.Vertical ?
TargetControl.ActualWidth :
TargetControl.ActualHeight;
}
}

So, in theory we may just be able to remove this initial code all together?
@michael-hawker michael-hawker added the bug Something isn't working label Jun 12, 2023
@michael-hawker
Copy link
Member Author

michael-hawker commented Jun 12, 2023 •
edited
Loading

Was able to write a test which I think demonstrates this issue:

image

Ah, and I found the key comment too:

// as if it's NaN we won't be able to manipulate it.

So that's why we need to set the Width somewhere... I think I'll do this check in the OnDragStarting though instead? As then layout should be complete as user manipulation is occuring.

@michael-hawker
Copy link
Member Author

I wonder if the original code re-used the Width field over the ActualWidth calculated from layout? As testing without setting it initially seems to be working fine... And the tests...pass

image

Basically, this should be better behavior, as now the ContentSizer doesn't change any initial behavior until it is manipulated by the user.

Opening a PR... :)

michael-hawker added a commit that referenced this issue Jun 12, 2023
@michael-hawker
Fixes #85 - ContentSizer manipulating component Width to Zero on load
4fd1e7d 
ContentSizer was overwriting Width to Zero causing initial layout problems of a component, instead this code is removed as it is not needed to maintain the same behavior. Added a test to validate the old behavior was incorrect and the new desired behavior should work.
michael-hawker added a commit that referenced this issue Jun 12, 2023
@michael-hawker
Fixes #85 - ContentSizer manipulating component Width to Zero on load
6106ce6 
ContentSizer was overwriting Width to Zero causing initial layout problems of a component, instead this code is removed as it is not needed to maintain the same behavior. Added a test to validate the old behavior was incorrect and the new desired behavior should work.
@michael-hawker michael-hawker mentioned this issue Jun 12, 2023
Merged
michael-hawker added a commit that referenced this issue Jun 13, 2023
@michael-hawker
Fixes #85 - ContentSizer manipulating component Width to Zero on load
7885900 
ContentSizer was overwriting Width to Zero causing initial layout problems of a component, instead this code is removed as it is not needed to maintain the same behavior. Added a test to validate the old behavior was incorrect and the new desired behavior should work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Toolkit 8.x
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #85 - ContentSizer manipulating component Width to Zero on load CommunityToolkit/Windows
1 participant
@michael-hawker