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

Proposal: Make ListViewItem stretch by default #1402

Closed
adrientetar opened this issue Oct 3, 2019 · 37 comments · Fixed by #3914
Closed

Proposal: Make ListViewItem stretch by default #1402

adrientetar opened this issue Oct 3, 2019 · 37 comments · Fixed by #3914
Assignees
Labels
area-Lists ListView, GridView, ListBox, etc feature proposal New feature proposal 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

Comments

@adrientetar
Copy link

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
@adrientetar adrientetar added the feature proposal New feature proposal label Oct 3, 2019
@YuliKl YuliKl added area-Lists ListView, GridView, ListBox, etc needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) labels Oct 3, 2019
@anawishnoff
Copy link
Contributor

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
Copy link
Author

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-dev-docs#1997

@robloo
Copy link
Contributor

robloo commented Dec 1, 2020

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.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Dec 1, 2020
@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Dec 2, 2020
@anawishnoff
Copy link
Contributor

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
Copy link
Contributor

robloo commented Dec 2, 2020

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.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Dec 2, 2020
@mdtauk
Copy link
Contributor

mdtauk commented Dec 2, 2020

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

@robloo
Copy link
Contributor

robloo commented Dec 2, 2020

@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
Copy link
Contributor

mdtauk commented Dec 2, 2020

@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
Copy link

nailuj29 commented Dec 2, 2020

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

@robloo
Copy link
Contributor

robloo commented Dec 2, 2020

@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
Copy link
Contributor

robloo commented Dec 2, 2020

@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
Copy link
Contributor

mdtauk commented Dec 2, 2020

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
Copy link
Contributor

robloo commented Dec 2, 2020

@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
Copy link
Contributor

mdtauk commented Dec 2, 2020

@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
Copy link
Contributor

robloo commented Dec 2, 2020

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

@anawishnoff
Copy link
Contributor

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.

@ghost ghost removed the needs-triage Issue needs to be triaged by the area owners label Dec 2, 2020
@robloo
Copy link
Contributor

robloo commented Dec 2, 2020

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

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Dec 2, 2020
@mdtauk
Copy link
Contributor

mdtauk commented Dec 2, 2020

@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
Copy link
Contributor

robloo commented Dec 2, 2020

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

@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Dec 3, 2020
@ranjeshj ranjeshj reopened this Dec 3, 2020
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Dec 3, 2020
@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Dec 3, 2020
@omikhailov
Copy link

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

  • This is one of the most annoying issues in all XAML frameworks and it must be fixed at any cost

  • ListView.HorizontalContentAlignment does not do anything because there is panel inside ListView (ListView.ItemsPanel), not just containers. This is useless property, but it is too late to change and reuse it.

  • As mentioned above, in case of adding new property you should be careful to do not attach to current implementation and specific type of the panel. ListView in UWP is simple list, but in WPF it was able to display grids with columns and other stuff, who knows what kind of panels and containers will be supported in the next versions of UWP's ListView?

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
Copy link
Contributor

robloo commented Dec 3, 2020

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

@ranjeshj
Copy link
Contributor

ranjeshj commented Dec 3, 2020

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
Copy link
Contributor

robloo commented Dec 3, 2020

@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 #1402 (comment).

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
Copy link
Contributor

ranjeshj commented Jan 6, 2021

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
Copy link
Contributor

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

@robloo
Copy link
Contributor

robloo commented Jan 6, 2021

@ranjeshj That's great to hear!

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

@marcelwgn
Copy link
Contributor

@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
Copy link
Contributor

robloo commented Jan 6, 2021

@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
Copy link
Contributor

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
Copy link
Contributor

robloo commented Jan 7, 2021

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
Copy link
Contributor

Given that the ListViewItem style is already in WinUI, you would probably just need to modify this line

<Setter Property="HorizontalContentAlignment" Value="Left" />

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

@robloo
Copy link
Contributor

robloo commented Jan 7, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Lists ListView, GridView, ListBox, etc feature proposal New feature proposal 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
None yet
Development

Successfully merging a pull request may close this issue.

10 participants