microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.26k stars 671 forks source link

Proposal: Make ListViewItem stretch by default #1402

Closed adrientetar closed 3 years ago

adrientetar commented 4 years ago

image

By default ListViewItem is left-aligned, which means that Grid columns won't stretch... It took me a while to find that I needed to add this snippet to make my DataTemplate root grid spans the whole space:

<ListView.ItemContainerStyle>
    <Style TargetType="ListViewItem">
        <Setter Property="HorizontalContentAlignment" Value="Stretch"/>
    </Style>
</ListView.ItemContainerStyle>
Capability Priority
Mention this snippet in the UWP docs (presumably here) Must
Make ListViewItem stretch by default Should
anawishnoff commented 4 years ago

Hello @adrientetar! ListViewItem has the HorizontalContentAlignment property default to Left to adhere to our style guidelines. Although I agree that what you're facing may be a common problem, that alone is not quite a strong enough argument to change this default value.

I do however agree with your suggestion about adding guidance for this situation into the docs, as its a common UI scenario for the ListView. I will look into the quickest way to get this done - you may have to open a separate issue in the docs repo.

adrientetar commented 4 years ago

that alone is not quite a strong enough argument to change this default value.

Right, I thought I'd point it out anyways because usually you partition with a Grid but in a ListViewItem it all suddenly gets pushed to the left. But probably not worth breaking backwards-compat for this change.

you may have to open a separate issue in the docs repo.

I'm not a big fan of that repo, as issues & PR don't get much attention there – if it helps though: MicrosoftDocs/windows-uwp#1997

robloo commented 3 years ago

Really think this needs to be adopted for WinUI 3 (see #3682). In the vast majority of cases the style needs to be stretched. That even follows Microsoft's design guidelines so Im not exactly sure what @anawishnoff was referring to. Its also what WPF does historically I believe.

anawishnoff commented 3 years ago

Hi @robloo, could you link me to the design guidelines you're referring to? I see on the other issue that in WPF the default horizontal alignment was center, and that using the left as default blocks folks from utilizing tools like star sizing in Grid. Are there any other use cases that you know of that are blocked by setting the default horizontal alignment to left?

Generally, we want to make sure that new changes we introduce are backwards compatible. This change would break a lot of apps in terms of their design (i.e. if they didn't specify a HorizontalAlignment in their ListViewItems but their design depends on it being the default Left), which is the biggest hurdle in my mind currently that is blocking this change from being approved.

robloo commented 3 years ago

When a ListViewItem is selected the entire row will be selected. Not just the left side or area with text, the entire row is selected as it is streched the full width of the ListView. This means, conceptually, a ListViewItem is already stretched and sets the design guidelines as such. Adding content to it that then isn't automatically stretched is quite counterintuitive for the vast majority of developers. Its also common to have action buttons in an item list. These buttons should be aligned to the right which requires stretching (would have to look for Microsoft examples but there are many I believe).

Talking about breaking changes, how would switching from Left to Stretch break existing apps? Any Child elements would still be on the left and left aligned by default in the parent. Only the parent container is now stretched. Any apps that customized this in some way also wouldn't break as their styles would still override any changes to the default.

mdtauk commented 3 years ago

Could there be a style bundled that would set it to Stretch? It would be better than making developers re-template.

robloo commented 3 years ago

@mdtauk I think the default style should stretch in all cases. Are you only suggesting a new style with a key? Or are you suggesting a new property too? So this can be set much easier to change if it needs to be customized.

@anawishnoff Also, I dont think WPF is center by default, where did you see that? Again, since we are just proposing changing the container stretch, I dont think this would change the look of existing apps as children still by default are left aligned. Only if they customized child element alignment in some way would things potentially be different (very rare and would arguable be a bug to set child as center for example and have it appear on left). Changing for WinUI 3 seems justifiable and would be a huge help for developers.

mdtauk commented 3 years ago

@mdtauk I think the default style should stretch in all cases. Are you only suggesting a new style with a key? Or are you suggesting a new property too? So this can be set much easier to change if it needs to be customized.

I would like the property to be exposed, but not sure if it is possible without locking in a particular panel as a container.

ListView .ItemContainerSizing maybe?

If a new property is too much of a change, then including a Style will at least offer a consistent "pre re-templated" solution for developers to use.

nailuj29 commented 3 years ago

@mdtauk I think exposing a property would be best, as some people might not want their ListView to stretch.

robloo commented 3 years ago

@mdtauk Exposing a property would definitely be a good thing. It shouldn't be necessary to re-template to do this. However, I would still like to change the default container alignment to stretch and think the concerns of breaking existing apps are unjustified for reasons listed above. If a new property is added it may be a good idea to name this closer to the HorizontalContentAlignment convention.

@nailuj29gaming Asking the questions Microsoft always asks... can you give any examples where developers actually wouldn't want to stretch the container? (Not talking about child elements, the container)

robloo commented 3 years ago

@anawishnoff I'm starting to collect examples (from Microsoft) where stretch is necessary and actually used in the illustrations. Again, it really is following the design guidelines.

https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.uielement.keyboardaccelerators?view=winrt-19041#Windows_UI_Xaml_UIElement_KeyboardAccelerators image

https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/lists image

https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/listview-and-gridview#listview Note that in this example I'm referring to the grouping header section separator (horizontal line). If a developer wants to do this themselves using a ListViewItem that isn't HitTestVisible, it wouldn't be possible as the line would never stretch the full width. image

@anawishnoff Checkmate!! :) https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/commanding#example

mdtauk commented 3 years ago

I believe when the control was moved from Windows Phone to Windows, the feeling was that the list items should not fill the width of a screen, and horizontal scrolling was being used a lot, so it made some sense to change the default.

But if its no longer the "received wisdom" but a change now could impact lots of apps - adding a property with a default unchanged would work - even if the default makes sense as Stretch.

robloo commented 3 years ago

@mdtauk I'm talking about changing the container only. Changing the container to stretch should never cause horizontal scrolling. Only if the ListViewItem itself has a width larger than the ListView width would scrolling ever apply -- which is no different than right now. Also, ListViewItem content within the container, again, would still be left aligned by default even if the container is now stretched. Changing the container size is very much independent of the child ListView content size - we have to keep those concepts separate.

mdtauk commented 3 years ago

@mdtauk I'm talking about changing the container only. Changing the container to stretch should never cause horizontal scrolling.

No I didn't say it would cause it, I was just saying Windows 8 used lots of horizontal scrolling UI and screens, and this may be why the stretch was no longer the default, so it would help.

robloo commented 3 years ago

@mdtauk I'm talking about changing the container only. Changing the container to stretch should never cause horizontal scrolling.

No I didn't say it would cause it, I was just saying Windows 8 used lots of horizontal scrolling UI and screens, and this may be why the stretch was no longer the default, so it would help.

Would those horizontal scrolling UI's be done in a GridView, not a ListView? A ListView is only for vertical scrolling.

mdtauk commented 3 years ago

@mdtauk I'm talking about changing the container only. Changing the container to stretch should never cause horizontal scrolling.

No I didn't say it would cause it, I was just saying Windows 8 used lots of horizontal scrolling UI and screens, and this may be why the stretch was no longer the default, so it would help.

Would those horizontal scrolling UI's be done in a GridView, not a ListView? A ListView is only for vertical scrolling.

Some controls and apps used a Horizontally aligned ListView. I can't recall the precise reasons stated at the time, only that the decision was made to change the default, and I believe it was before Windows 8 released.

There are endless examples of people asking why their ListViews are not filling the width, so making it easier is worth doing - but changing the default that has existed for many, many years (whilst it makes total sense) may cause issues, where a new property could use the current stretching as it's default, but allow a simple way to set it without touching the template.

A property means Intellisense can alert devs about it - but if thats not going to be considered - a Style or Lightweight style could be added.

robloo commented 3 years ago

@mdtauk Changing the container shouldn't be much of an issue.:

We seem to be denying real benefits for some hypothetical downsides that may not exist in real apps. Additionally, for a big transition like WinUI 3 where there are other breaking changes I don't think this is a problem at all. Common-sense changes like this are necessary to improve platforms over time. That said, I'm all for adding a property as well -- it is a great idea.

@adrientetar Since you were the one to close this, would you mind re-opening?

anawishnoff commented 3 years ago

Thanks to you all for the lively conversation and for your effort in driving this! I need to clarify some things, as I believe I was a bit confused previously.

@robloo, due to the nature of this issue I was under the assumption that you were suggesting changing the ListViewItem.HorizontalContentAlignment property to be Stretch by default. After reading your latest comments, I see that you're actually suggesting changing the ListView.HorizontalContentAlignment property to be Stretch by default? Is this what you mean when you refer to the "container"? Perhaps I'm misunderstanding, but I'm not too clear on what exactly would visually change if you set ListView.HorizontalContentAlignment to Stretch.

I messed around with a few examples in Visual Studio and can see that adding this code as mentioned above creates some visual changes:

<ListView.ItemContainerStyle>
    <Style TargetType="ListViewItem">
        <Setter Property="HorizontalContentAlignment" Value="Stretch"/>
    </Style>
</ListView.ItemContainerStyle>

But there aren't any noticeable visual changes when I set ListView.HorizontalContentAlignment property to Stretch. Could you expand on this? Thanks!

@mdtauk The idea of a property is definitely enticing as it would make it an opt-in change rather than risking breaking anything. Would this property basically just be a quicker way to set the following style (or something along these lines)?

 <ListView.Style>
    <Style TargetType="ListView">
           <Setter Property="HorizontalContentAlignment" Value="Stretch" />
    </Style>
</ListView.Style>
mdtauk commented 3 years ago

@anawishnoff I think this is the issue being discussed in a Blog post Tip: Stretching list items in UWP, please correct me if I am wrong @robloo

And it may also be the same as this Docs issue you responded too Ana Mention how to make ListViewItem stretch horizontally

robloo commented 3 years ago

@robloo, due to the nature of this issue I was under the assumption that you were suggesting changing the ListViewItem.HorizontalContentAlignment property to be Stretch by default. After reading your latest comments, I see that you're actually suggesting changing the ListView.HorizontalContentAlignment property to be Stretch by default? Is this what you mean when you refer to the "container"? Perhaps I'm misunderstanding, but I'm not too clear on what exactly would visually change if you set ListView.HorizontalContentAlignment to Stretch

Yes, I don't think we are on the same page yet. I'm not talking about changing ANY of the properties you mentioned in the above quote. (more on that later). Basically, neither of these properties map to the ListViewItem's container which is actually what needs to be changed. Describing how the visual tree is constructed for the ListView needs to be explained I think. My understanding is:

Most developers, don't realize that the ListViewItem's content is actually added to a container and then the container to the ListView. The container is necessary for many things -- selection background coloring for example. However, it's the container content alignment that was changed to Left in UWP instead of Stretch for reasons I can't explain.

ListView -> ListView.HorizontalContentAlignment ListViewItem -> ListViewItem.HorizontalContentAlignment property ListViewItemContainer -> there is no property and we must re-template

But there aren't any noticeable visual changes when I set ListView.HorizontalContentAlignment property to Stretch. Could you expand on this? Thanks!

Sure, I'll expand on it. You are essentially asking me to explain Microsoft tech to Microsoft though and I'm not paid like some of your engineers to do that. The code example you gave is exactly what I'm talking about. You are changing the container style to stretch. Here is more real-life code doing the same taking from an app in the store:

            <ListView
                x:Name="MessageListView"
                VerticalAlignment="Stretch"
                HorizontalAlignment="Stretch"
                HorizontalContentAlignment="Stretch"
                ItemTemplateSelector="{StaticResource MessageListTemplateSelector}">
                <ListView.ItemContainerStyle>
                    <Style
                        TargetType="ListViewItem"
                        BasedOn="{StaticResource ListViewItemRevealStyle}">
                        <Setter
                            Property="HorizontalContentAlignment"
                            Value="Stretch" />
                    </Style>
                </ListView.ItemContainerStyle>
            </ListView>

We are setting the ListView.ItemContainerStyle (the ListViewItemContainer) NOT the ListView.HorizontalContentAlignment (which does who knows what, if anything). Setting the container style here allows all ListViewItem content to take up the full width if it is stretched. It doesn't change the alignment of the ListViewItem content itself. If the ListViewItem is left aligned (which is the default) you won't notice any changes. You are simply stretching the container and making the full width available to the content.

With this code you are also understanding my fundamental point about breaking changes concerns. There aren't any noticeable changes in the vast majority of cases (and guaranteed no changes if devs were using defaults). Hopefully we are on the same page now but if not let me know.

Some additional points:

anawishnoff commented 3 years ago

Thank you both for the clarification, I apologize for the confusion and your proposal makes a lot more sense now that I know you're talking about ListViewItemContainer rather than ListView.

Now that I understand what's being changed, I think adding a simple property to do this would be the best path forward, rather than just changing the default. And you have also convinced me on the breaking changes point - thanks for ironing that out.

Pinging @ranjeshj to see if he has any concerns with moving forward with adding a property to address this. My thinking would be that the property should be on ListView itself for maximum discoverability. As for naming, perhaps ListView.ItemContainerHorizontalContentAlignment? We can get into more details and such if this moves on to the spec process.

robloo commented 3 years ago

@anawishnoff We should still change the default. I think you are unilaterally making this decision without understanding how much benefit it will be to developers. Both existing and those coming over from WPF for WinUI 3. I don't think it's in my interest to drop this until I'm sure it has been widely discussed. My biggest fear is we have people making WinUI decisions that have much less experience in XAML than those using it. Pinging @StephenLPeters and @jevansaks as well.

My last point was we may not need another property. ListView.HorizontalContentAlignment may be doing nothing and could be re-purposed.

mdtauk commented 3 years ago

@robloo You can't assume everyone working in WinUI to have been familiar with Xaml as long as others, or yourself. Please try to not get frustrated and give people the benefit of your knowledge in a polite and helpful way.

Any change from the current defaults will be viewed with skepticism, especially as it is a change that was made some years ago, for possibly good reasons back then.

The fact that it has been a pain point for people in the past, and differs from WPF - are good reasons to change it, but its not a decision one person will make, and I believe @ranjeshj may be a key decision maker. I see his name associated with various big mergers in the repo.

robloo commented 3 years ago

@mdtauk Yes, my frustration over the years has become visible hasn't it. You are correct there are more polite ways to phrase things. I am intentionally putting up resistance in the hope of gaining attention and changes though. More often than not perfect politeness doesn't get things done. You must put up resistance or things will continue as they have been on the proverbial path of least resistance. Of course if you put up too much resistance people ignore you as well -- difficult to balance through remote communication. I am putting up resistance and being persistent -- strongly when needed -- in the hopes of encouraging changes. But please don't look at my 'impolite' comments (which is relative and cultural) as anything more than that. In business there are times to be perfectly polite and times not to be.

omikhailov commented 3 years ago

I had refused to register on github for many years prior to this discussion. Here is my opinion:

This way, original proposal is the best way to fix this confusing behaviour of the ListView. Default style of the ListViewItem must have Stretch for HorizontalContentAlignment.

It's hard to say why it was initially set to Left, probably it was done for better performance, but today such tradeoff is irrelevant.

robloo commented 3 years ago

@omikhailov Good points about the items panel I forgot to factor in.

ranjeshj commented 3 years ago

I agree that this is very undiscoverable and would be great to fix in way that is not breaking. Changing the default would still cause a lot of pain for existing apps that expect the current behavior (and to hunt for the undiscoverable solution to fix it) Either a new API or repurposing an unused API on ListView sounds fine to me. @MikeHillberg do you know the reasoning for the current default ?

robloo commented 3 years ago

@ranjeshj Can you elaborate on how this would break existing apps? As its only changing the container size I think the risks are very low as commented https://github.com/microsoft/microsoft-ui-xaml/issues/1402#issuecomment-737396498.

WinUI 3 is also a perfect place to slip this in with other much more significant breaking changes. We shouldn't be fearing changes with clear benefits like this and its unarguably the right time to do this with WinUI 3.

ranjeshj commented 3 years ago

Discussed with some folks internally and I think it makes sense to just update this in the WinUI2 style and let it flow to WinUI3.

Adding this to the ListView style should do the trick <Setter Property="HorizontalContentAlignment" Value="Stretch"/>

marcelwgn commented 3 years ago

@ranjeshj Is this good to work on then? Would you like me to create a PR for this?

robloo commented 3 years ago

@ranjeshj That's great to hear!

@chingucoding If you don't I will. I'm glad to finally see this closed.

marcelwgn commented 3 years ago

@robloo If you want to create the PR for this, sure go for it (assuming it's fine with the team which it seems).

robloo commented 3 years ago

@chingucoding I didn't mean to take your steam on this. Feel free to add it yourself!

There is also no default style for this in WinUI at this point. I'm not sure how to add that and make sure it's properly registered. Additionally, this isn't the ListView style itself.: it's the ListViewItemRevealStyle style as demonstrated below:

<ListView.ItemContainerStyle>
  <Style
    TargetType="ListViewItem"
    BasedOn="{StaticResource ListViewItemRevealStyle}">
      <Setter
        Property="HorizontalContentAlignment"
        Value="Stretch" />
    </Style>
</ListView.ItemContainerStyle>
marcelwgn commented 3 years ago

And we have to override this in the ListView template? Shouldn't it be enough to add the modified reveal style to WinUI 2 then?

Also, please if you want to work on this feel free to do so!

robloo commented 3 years ago

And we have to override this in the ListView template? Shouldn't it be enough to add the modified reveal style to WinUI 2 then?

I was saying, no, we don't modify the ListView template (as Ranjesh implied). Yes, we should just modify the ListViewItemRevealStyle. The copy pasted code was just an example for how the style is set in UWP apps -- probably a bad example. I didn't mean this as an example of what the default style will be.

I do not know how styles are picked up by the WinUI library though -- I'm not sure how they are registered. I have only ever modified existing files not created new ones.

Here is what I expect the xaml to actually look like.

    <Style TargetType="ListViewItem" BasedOn="{StaticResource ListViewItemRevealStyle}" />

    <Style TargetType="ListViewItem" x:Key="ListViewItemRevealStyle">
        <Setter Property="FontFamily" Value="{ThemeResource ContentControlThemeFontFamily}" />
        <Setter Property="FontSize" Value="{ThemeResource ControlContentThemeFontSize}" />
        <Setter Property="Background" Value="{ThemeResource ListViewItemBackground}" />
        <Setter Property="Foreground" Value="{ThemeResource ListViewItemForeground}" />
        <Setter Property="TabNavigation" Value="Local" />
        <Setter Property="IsHoldingEnabled" Value="True" />
        <Setter Property="Padding" Value="12,0,12,0" />
        <Setter Property="HorizontalContentAlignment" Value="Stretch" />
        <Setter Property="VerticalContentAlignment" Value="Center" />
        <Setter Property="MinWidth" Value="{ThemeResource ListViewItemMinWidth}" />
        <Setter Property="MinHeight" Value="{ThemeResource ListViewItemMinHeight}" />
        <Setter Property="AllowDrop" Value="False" />
        <Setter Property="UseSystemFocusVisuals" Value="{StaticResource UseSystemFocusVisuals}" />
        <Setter Property="FocusVisualMargin" Value="0" />
        <Setter Property="Template">
            <Setter.Value>
                <ControlTemplate TargetType="ListViewItem">
                    <!-- removed for example -->
                </ControlTemplate>
            </Setter.Value>
        </Setter>
    </Style>
marcelwgn commented 3 years ago

Given that the ListViewItem style is already in WinUI, you would probably just need to modify this line https://github.com/microsoft/microsoft-ui-xaml/blob/2f6f77ca5cb9b9d206fe0a3922219de96d9024df/dev/CommonStyles/ListViewItem_themeresources.xaml#L164

as ListView just pick this style up as we registered it as the default style for ListViewItem.

robloo commented 3 years ago

Ah, I did not know of the common styles directory, thanks!