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.

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

StackPanel applies Spacing to Collapsed items #916

Closed
bschoepke opened this issue Jun 21, 2019 · 31 comments
Closed

StackPanel applies Spacing to Collapsed items #916

bschoepke opened this issue Jun 21, 2019 · 31 comments
Labels
area-Layouts bug Something isn't working needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team
Milestone

Comments

@bschoepke
Copy link

bschoepke commented Jun 21, 2019

Describe the bug
StackPanel applies Spacing to Collapsed child items, which means a Collapsed item causes whitespace of 2x the spacing value.

Steps to reproduce the bug

    <StackPanel Spacing="24" BorderThickness="2" BorderBrush="Navy" Padding="24">
        <StackPanel.Resources>
            <Style TargetType="Border">
                <Setter Property="BorderThickness" Value="1"/>
                <Setter Property="BorderBrush" Value="{ThemeResource SystemControlBackgroundBaseHighBrush}"/>
            </Style>
        </StackPanel.Resources>
        <Border>
            <TextBlock Text="..."/>
        </Border>

        <Border>
            <TextBlock Text="Hello"/>
        </Border>

        <!-- Expected: 24px whitespace between Hello and World. Observed: 48px. -->
        <Border Visibility="Collapsed">
            <TextBlock Text="..." />
        </Border>

        <Border>
            <TextBlock Text="World"/>
        </Border>

        <Border>
            <TextBlock Text="..."/>
        </Border>
    </StackPanel>

Expected behavior
Collapsed elements should not be considered in Spacing calculations.

Screenshots
image

Version Info
C# UWP app targeting 1903 SDK

NuGet package version:
n/a

Windows 10 version Saw the problem?
Insider Build (18913) Yes
May 2019 Update (18362) ?
October 2018 Update (17763) ?
April 2018 Update (17134) ?
Fall Creators Update (16299) ?
Creators Update (15063) ?
Anniversary Update (14393) ?
Device form factor Saw the problem?
Desktop Yes
Mobile ?
Xbox ?
Surface Hub ?
IoT ?
@msft-github-bot msft-github-bot added this to Needs triage in Controls Triage Jun 21, 2019
@chrisglein chrisglein added this to Needs triage in Framework Triage via automation Jun 21, 2019
@chrisglein chrisglein removed this from Needs triage in Controls Triage Jun 21, 2019
@chrisglein chrisglein added the needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) label Jul 15, 2019
@chrisglein chrisglein moved this from Needs triage to Backlog in Framework Triage Jul 15, 2019
@chrisglein chrisglein added the bug Something isn't working label Jul 15, 2019
@jevansaks jevansaks added area-Framework needs-triage Issue needs to be triaged by the area owners needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) and removed needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) needs-triage Issue needs to be triaged by the area owners labels Nov 7, 2019
@robloo
Copy link
Contributor

robloo commented Jun 11, 2020

I just ran across this one myself. Definitely would be a big help to fix this for cases where items are dynamically shown/hidden in a StackPanel.

@MartinZikmund
Copy link
Contributor

I would definitely love to see this behavior adjusted. Maybe it could be controlled by a separate property like SkipCollapsedSpacing. Similarly it would be useful for Grid where it could be used to skip spacing for empty columns/rows.

@robloo
Copy link
Contributor

robloo commented Dec 29, 2020

This is more of a bug. I wouldn't advocate adding a new property to control this. Concerns about changing existing apps would be trivial. The change is non-breaking and would only slightly affect the look of some (an extremely small number) of apps.

@MartinZikmund
Copy link
Contributor

@robloo I am afraid people who come across this have actually employed some "hacks" to work around this (like negative margins), so it would be a breaking change. But it is probably an acceptable one. For Grid on the other hand, it could be a new (and useful) behavior.

@robloo
Copy link
Contributor

robloo commented Dec 29, 2020

Non-breaking to me means the app will still compile, build and run without errors or crash. Of course visually these are differences but there are also differences when styling is updated from time to time. I agree that it is acceptable reguardless.

@duraz0rz
Copy link

Just ran across this when trying out the Spacing property. We have a TextBlock inside a StackPanel that we dynamically show/hide, and it would be nice to use this property vs setting Margin on every child element in the StackPanel. Any updates?

@ZodmanPerth
Copy link

I'm adding my +1 to this.

@tomtom-m
Copy link

tomtom-m commented Dec 3, 2021

Since there doesn't seem to be an official fix for the problem, I created a new implementation which inherits from StackPanel. My version uses margins to create spacing between items in the StackPanel, but only for those items which are visible.

Full implemenation: StackPanelWithSpacing

private void SetSpacingForChildren(int spacing)
        {
            for (int i = 0; i < this.Children.Count; i++)
            {
                if (this.Children[i] is FrameworkElement element
                    && element.Visibility == Visibility.Visible)
                {
                    var halfSpacing = spacing / 2;
                    var topSpacing = i == 0 ? 0 : halfSpacing;
                    element.Margin = new Thickness(element.Margin.Left, element.Margin.Top + topSpacing, element.Margin.Right, element.Margin.Bottom + halfSpacing);
                }
            }
        }

@brandon3343
Copy link

brandon3343 commented Jul 22, 2022

two years has passed, issue should be closed if not planned.

@MartinRichards23
Copy link

I'd like to see this change, I've never been able to use the Spacing property due to this, and I would use it often otherwise.

@LucaCris
Copy link

This is a "one minute fix" (replicating @tomtom-m code). Please...

@Odinodin
Copy link

Odinodin commented Dec 5, 2022

I wasted some time on this today, is there any chance this bug will be fixed after 3.5 years?

@trungnt2910
Copy link

This is the nth WinUI bug that I've encountered today, all of which has been opened for a few years and labeled with needs-winui-3.

With WinUI3 out for a while now, when will this issue (and many others with needs-winui-3) be finally fixed?

@robloo
Copy link
Contributor

robloo commented Dec 13, 2022

@trungnt2910 Begin transition to other UI frameworks. Microsoft is killing this off for anyone but the OS team and C++ apps. I would even go as far as to say it will never be open sourced (contrary to what they will publicly state) so the community will never be able to fix issues like these. If you are using C# Avalonia will be dominant in a few years, it is already dominant in most scenarios.

Microsoft is repeating the same failures here with WinUI3 we've seen many times before. #3639

@trungnt2910
Copy link

I already tried Avalonia, probably the best framework for desktop-only development, but not most scenarios.

Still I'm developing a cross-platform application (Desktop, Mobile, Web), and I'm using the Uno Platform for this. On Windows, I rely on WinUI3.

@bogdan-patraucean
Copy link

This is a basic feature that should work by default. We deserve an update on this. I encountered the same issue and it's just annoying to see it doesn't work as expected.

@bogdan-patraucean
Copy link

bogdan-patraucean commented Mar 12, 2023

@tomtom-m I've allowed myself to improve your solution as it doesn't take 2 aspects into consideration:

  • applying the spacing when the orientation is Horizontal
  • not adding spacing after the last visible element as it's not needed (only elements in between should have spacing)
var lastVisibleIndex = Children.IndexOf(Children.LastOrDefault(c => c.Visibility is Visibility.Visible));

for (int i = 0; i < Children.Count - 1; i++)
{
    if (Children[i] is FrameworkElement child && child.Visibility is Visibility.Visible && i != lastVisibleIndex)
    {
        child.Margin = new Thickness(child.Margin.Left, child.Margin.Top, Orientation is Orientation.Horizontal ? gap : child.Margin.Right, Orientation is Orientation.Vertical ? gap : child.Margin.Bottom);
    }
}

@LucaCris
Copy link

Correction, @bogdan-patraucean :

Orientation is Orientation.Horizontal ? child.Margin.Right + gap : child.Margin.Right

and the same for the other side...

@LucaCris
Copy link

LucaCris commented Jun 8, 2023

Any updates? Almost 4 years since this issue was opened... I'm testing WinUI3, but this kind of unexpected behavior are annoying and discouraging for the adoption of this library.

Do not exagerate... The library is very good. There are some minor problems only.

@robloo
Copy link
Contributor

robloo commented Jun 8, 2023

That wasn't an exaggeration by any means. It is an example of pain point that has never been fixed (but should have been easy to resolve). It's why, among many other things, many companies (including mine) dropped UWP/WinUI. Even MAUI is adding a WPF backend because WinUI 3 future is in question.

@bogdan-patraucean
Copy link

bogdan-patraucean commented Jun 8, 2023

Any updates? Almost 4 years since this issue was opened... I'm testing WinUI3, but this kind of unexpected behavior are annoying and discouraging for the adoption of this library.

Do not exagerate... The library is very good. There are some minor problems only.

There are some really serious problems. Indeed, maybe this isn't one of them but they add up. You can't event set a Min or Max width and height for a Window. I myself have a lot of issues opened that haven't been fixed yet. The team working on this is small, and their effort is sometimes redirected in other parts than fixing the library, like File Explorer for example.

@trungnt2910
Copy link

@robloo

Even MAUI is adding a WPF backend because WinUI 3 future is in question.

A bit off-topic, but where did you get this information?

@fabianoriccardi
Copy link

fabianoriccardi commented Jun 8, 2023

A bit off-topic, but where did you get this information?

I'm curious too, it is not the first time I read similar comment un WinUI team.

You can't event set a Min or Max width and height for a Window.

Moreover, you cannot either set the width/height of the Window, you have to get AppWindow to do it. Nothing near to an issue, but it seems I'm not using a "native" framework.

It is an example of pain point that has never been fixed.

Exactly, there are many bugs also on TreeView (I have performed a query and there are 73 open issues, I have personally hit few of them), and this is a quite basic component, TreeView exists since WinForms and even before.

Now, I'm not really blaming the developers, I know that if man-power and budget is not enough the result cannot be satisfying, but I would understand if investing in this framework will pay in the future.

Currently I'm not giving up, just questioning :)

UPDATE: even today, during daily activities, I have met 2 well known annoyances (or issues) about DialogContent (March 2019) and about NumberBox (August 2022).

@robloo
Copy link
Contributor

robloo commented Jun 8, 2023

A bit off-topic, but where did you get this information?

At this point let's just say rumors from the BUILD conference.

@bpulliam bpulliam added team-Controls Issue for the Controls team and removed team-Framework labels Aug 22, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Aug 22, 2023
@bpulliam bpulliam removed the needs-triage Issue needs to be triaged by the area owners label Aug 29, 2023
@JJBrychell JJBrychell added the fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. label Jan 26, 2024
@JJBrychell
Copy link

Fix is in to not add spacing for collased items. It is scheduled for 1.5.

@bogdan-patraucean
Copy link

@JJBrychell awesome news, I can finally ditch my custom Stackpanel.

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Jan 26, 2024
MartinZikmund added a commit to unoplatform/uno that referenced this issue Feb 26, 2024
@bpulliam bpulliam added this to the WinAppSDK 1.5 milestone Feb 29, 2024
@bpulliam bpulliam removed needs-triage Issue needs to be triaged by the area owners fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. labels Feb 29, 2024
MartinZikmund added a commit to unoplatform/uno that referenced this issue Mar 3, 2024
@legistek
Copy link

legistek commented May 4, 2024

Grid has this issue too.

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label May 4, 2024
@codendone
Copy link
Contributor

See #9491 for Grid.

@codendone codendone removed the needs-triage Issue needs to be triaged by the area owners label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Layouts bug Something isn't working needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team
Projects
No open projects
Development

No branches or pull requests