microsoft-ui-xaml icon indicating copy to clipboard operation
microsoft-ui-xaml copied to clipboard

StackPanel applies Spacing to Collapsed items

Open bschoepke opened this issue 6 years ago • 9 comments

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 ?

bschoepke avatar Jun 21 '19 21:06 bschoepke

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.

robloo avatar Jun 11 '20 02:06 robloo

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.

MartinZikmund avatar Dec 28 '20 20:12 MartinZikmund

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.

robloo avatar Dec 29 '20 17:12 robloo

@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.

MartinZikmund avatar Dec 29 '20 19:12 MartinZikmund

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.

robloo avatar Dec 29 '20 20:12 robloo

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?

duraz0rz avatar Mar 26 '21 18:03 duraz0rz

I'm adding my +1 to this.

ZodmanPerth avatar Jul 01 '21 06:07 ZodmanPerth

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);
                }
            }
        }

tomtom-m avatar Dec 03 '21 11:12 tomtom-m

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

brandon3343 avatar Jul 22 '22 04:07 brandon3343

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.

MartinRichards23 avatar Oct 17 '22 10:10 MartinRichards23

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

LucaCris avatar Nov 11 '22 14:11 LucaCris

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

Odinodin avatar Dec 05 '22 09:12 Odinodin

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?

trungnt2910 avatar Dec 12 '22 10:12 trungnt2910

@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. https://github.com/microsoft/microsoft-ui-xaml/issues/3639

robloo avatar Dec 13 '22 02:12 robloo

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.

trungnt2910 avatar Dec 13 '22 02:12 trungnt2910

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 avatar Mar 12 '23 14:03 bogdan-patraucean

@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);
    }
}

bogdan-patraucean avatar Mar 12 '23 18:03 bogdan-patraucean

Correction, @bogdan-patraucean :

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

and the same for the other side...

LucaCris avatar Mar 23 '23 14:03 LucaCris

@LucaCris the idea is to place the spacing only when the orientation is set to that particular one, otherwise the default Margin can be applied, not viceversa. Just test it, it works.

bogdan-patraucean avatar Mar 25 '23 19:03 bogdan-patraucean

@LucaCris the idea is to place the spacing only when the orientation is set to that particular one, otherwise the default Margin can be applied, not viceversa. Just test it, it works.

The implicit item margin must be present every time. Plus the gap when needed...

LucaCris avatar Apr 01 '23 16:04 LucaCris

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.

fabianoriccardi avatar Jun 06 '23 16:06 fabianoriccardi

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.

LucaCris avatar Jun 08 '23 12:06 LucaCris

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.

robloo avatar Jun 08 '23 12:06 robloo

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.

bogdan-patraucean avatar Jun 08 '23 12:06 bogdan-patraucean

@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?

trungnt2910 avatar Jun 08 '23 13:06 trungnt2910

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).

fabianoriccardi avatar Jun 08 '23 17:06 fabianoriccardi

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

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

robloo avatar Jun 08 '23 17:06 robloo

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

JJBrychell avatar Jan 26 '24 21:01 JJBrychell

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

bogdan-patraucean avatar Jan 26 '24 21:01 bogdan-patraucean

Grid has this issue too.

legistek avatar May 04 '24 17:05 legistek