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

TabView Control #2565

Merged
merged 94 commits into from Oct 29, 2018
Merged

Conversation

michael-hawker
Copy link
Member

@michael-hawker michael-hawker commented Oct 16, 2018

Issue: #353

image

PR Type

What kind of change does this PR introduce?

  • Feature

What is the new behavior?

Add TabView control to support various scenarios for both WPF Tab Control and Edge/Document style tab scenarios. Supports in-line items and data sources. Can be used to host the content and for just a tab strip.

Supports two styles, by default, should behave like a modern WPF TabControl and provide a simple way to switch between different content areas with simple tabs. The provided TabViewDocumentStyle sets up a more Edge/Document style tab setup where tabs have more defined sizes and adjust based on the space available.

Different areas and templates are available for advanced customization and placement scenarios. The current sample page provides a sample of most of these, but should probably be cleaned-up a bit for a singular scenario.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

Known Issues to fix:

  • Inline Items trying to drive another container causes crash
  • Specifying Blank ItemTemplate (or any) doesn't override content when using inline items
  • Header with no text causes crash (Find Ancestor?)
  • Sample loading twice is messing with finding the AddTabButton
  • High contrast check
  • Need to test some drag-drop scenarios messing with content presenter.
  • Check using in designer.

Out of current scope

  • TabStripPlacement property

…yle' provide Style values for customization.
@michael-hawker
Copy link
Member Author

@skendrot thanks the for the feedback. I didn't know you could middle-click tabs to close them! I won't have much time to investigate that today, so we can add a bug for it after.

I'll take a quick look at the in-app notification. I'll just put it in the sample page so it doesn't muck-up the example xaml. I've fixed the crash.

We tweaked our default behavior to keep the SelectedTabWidth as the Max; however, to replicate Edge's exact behavior you can set the behavior to Equal, remove the SelectedTabWidth, and set the MaxWidth in the TabViewItem style to 200 (I'm having trouble getting the sample xaml editor to do this, but tested it modifying the default style). It exposes a bug in my required calculation though currently, I'll have to look into that.

@michael-hawker
Copy link
Member Author

Added Docs PR here

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Approval contingent on addressing the comments

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/samples.json Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.SampleApp/SamplePages/samples.json Outdated Show resolved Hide resolved
@nmetulev
Copy link
Contributor

nmetulev commented Oct 27, 2018

@skendrot, it be amazing if we can get this merged in for 5.0, ideally by today. Could you make sure all your comments were addressed and approve if so?

Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabView.cs Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabView.cs Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabView.cs Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabView.cs Outdated Show resolved Hide resolved

if (SelectedItem == null)
{
_tabContentPresenter.Content = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed if we're using TemplateBinding. The TabContentPresenter should bind to the SelectedItem and the ContentTemplate. The TabView should have a ContentTemplate property to set

Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabViewItem.cs Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabViewItem.cs Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabViewItem.cs Outdated Show resolved Hide resolved
Microsoft.Toolkit.Uwp.UI.Controls/TabView/TabViewItem.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@skendrot skendrot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TabView does not appear to allow for binding the ItemsSource and defining Styles either implicitly, or through the ItemContainerStyle.

Example:
Given the following layout:

    <Grid>
        <Grid.Resources>
            <Style TargetType="controls:TabViewItem">
                <Setter Property="IsClosable" Value="True"/>
                <Setter Property="Header" Value="{Binding Title}"/>
                <Setter Property="Content" Value="{Binding Data}"/>
            </Style>
        </Grid.Resources>
        <controls:TabView ItemsSource="{x:Bind Items}">
            
        </controls:TabView>
    </Grid>

And the code to match:

            Items = new List<Item>
            {
                new Item { Title = "Item 1", Data="The info for item 1"},
                new Item { Title = "Item 2", Data="The info for item 2"},
                new Item { Title = "Item 3", Data="The info for item 3"},
                new Item { Title = "Item 4", Data="The info for item 4"}
            };

The tabs are not rendered properly at all.
image

Even worse if you define the style in the ItemContainerStyle of the TabView

    <Grid>
        <Grid.Resources>
            <Style TargetType="controls:TabViewItem">
                <Setter Property="IsClosable" Value="True"/>
                <Setter Property="Header" Value="{Binding Title}"/>
                <Setter Property="Content" Value="{Binding Data}"/>
            </Style>
        </Grid.Resources>
        <controls:TabView ItemsSource="{x:Bind Items}">
            <controls:TabView.ItemContainerStyle>
                <Style TargetType="controls:TabViewItem">
                    <Setter Property="IsClosable" Value="True"/>
                    <Setter Property="Header" Value="{Binding Title}"/>
                    <Setter Property="Content" Value="{Binding Data}"/>
                </Style>
            </controls:TabView.ItemContainerStyle>
        </controls:TabView>
    </Grid>

image

@JustinXinLiu
Copy link
Contributor

@skendrot <Setter Property="Content" Value="{Binding Data}"/> UWP doesn't support Binding in Setter.

@skendrot
Copy link
Contributor

@JustinXinLiu Dangit, that always gets me! UWP needs to catch up with WPF!

Copy link
Contributor

@skendrot skendrot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing tabs in which items come from a non-INotifyCollectionChanged collection does not remove the visual component
Example:
Xaml:

        <controls:TabView ItemsSource="{x:Bind Items}">
            <controls:TabView.ItemContainerStyle>
                <Style TargetType="controls:TabViewItem">
                    <Setter Property="IsClosable" Value="True"/>
                </Style>
            </controls:TabView.ItemContainerStyle>
        </controls:TabView>

Code:

            Items = new List<Item>
            {
                new Item { Title = "Item 1", Data="The info for item 1"},
                new Item { Title = "Item 2", Data="The info for item 2"},
                new Item { Title = "Item 3", Data="The info for item 3"},
                new Item { Title = "Item 4", Data="The info for item 4"}
            };

Click on any close button and the item is removed from the backing collection but it is not removed from the visual tree

Copy link
Contributor

@skendrot skendrot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log an issue about closing tabs with a non-INotifyCollectionChanged and we can call it good

@michael-hawker
Copy link
Member Author

Yeah, I call out suggesting ObservableCollection in the docs, but I can see if I can fix it for non-observable in the control, at least if you use the button I should do it. But if they remove something from the collection, we can't tell.

@michael-hawker michael-hawker merged commit 4c24a31 into CommunityToolkit:master Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants