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

Tabs And NavigationRail Styles for WPF TabControl #2026

Merged
merged 10 commits into from Sep 18, 2020

Conversation

HClausing
Copy link
Contributor

@HClausing HClausing commented Aug 17, 2020

Tabs:
image

Navigation Rail:
image

Source:
Tabs
NavigationRail

@seasonyuu
Copy link

It's pretty nice !

Copy link
Member

@Keboo Keboo left a comment

Choose a reason for hiding this comment

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

This is really great work! Thank you for doing this!

A few comments:

  • It would be great to pull the generic TabControl styles out for now, and submit a follow up PR. TabControl styles are a bit of a hot button topic, and I need to get more approval before that portion merges.

Some other optional items that could also be done later:

  • Making the Divider optional. It looks like we are using a drop shadow for it now, but the spec appears to just be a simple line.
  • Include in demo app, a sample using text labels. Ideally this show default with text wrapping. It might be nice to also have the demo show the text label only appearing on the selected tab (Selected text labels example).
  • Demo app sample showing badge control use in the TabItem Header.
  • Demo app showing it used for bottom navigation

MainDemo.Wpf/NavigationRail.xaml Outdated Show resolved Hide resolved
MainDemo.Wpf/NavigationRail.xaml Outdated Show resolved Hide resolved
MainDemo.Wpf/NavigationRail.xaml Outdated Show resolved Hide resolved
@HClausing
Copy link
Contributor Author

HClausing commented Sep 5, 2020

This is really great work! Thank you for doing this!

A few comments:

  • It would be great to pull the generic TabControl styles out for now, and submit a follow up PR. TabControl styles are a bit of a hot button topic, and I need to get more approval before that portion merges.

Some other optional items that could also be done later:

  • Making the Divider optional. It looks like we are using a drop shadow for it now, but the spec appears to just be a simple line.
  • Include in demo app, a sample using text labels. Ideally this show default with text wrapping. It might be nice to also have the demo show the text label only appearing on the selected tab (Selected text labels example).
  • Demo app sample showing badge control use in the TabItem Header.
  • Demo app showing it used for bottom navigation

Thanks for the review!

Related to the Divider tip, I've added it. About the drop shadow, I made it inheritable by materialDesign:ShadowAssist.ShadowDepth, with a Depth2 as (old) default. Now is set to Depth0):

image

@Keboo
Copy link
Member

Keboo commented Sep 11, 2020

This is looking really nice and I would like to include it in the 3.x release.

A few things that are needed for this to merge:

  • It appears that the background/foreground of the items does not follow inherited theme brushes. I would expect the foreground/background to track with whatever was set on the parent window.
    image
  • There appears to be a rendering issue with the shadows/separator.
    ddae27eb-bf60-46c1-82dc-8791363f1389
  • The non-rail styles should get pulled out. For the initial release it should only be the navigation rail bits.

@HClausing
Copy link
Contributor Author

HClausing commented Sep 11, 2020

This is looking really nice and I would like to include it in the 3.x release.

A few things that are needed for this to merge:

  • It appears that the background/foreground of the items does not follow inherited theme brushes. I would expect the foreground/background to track with whatever was set on the parent window.
    image
  • There appears to be a rendering issue with the shadows/separator.
    ddae27eb-bf60-46c1-82dc-8791363f1389
  • The non-rail styles should get pulled out. For the initial release it should only be the navigation rail bits.

Thanks for the review! We are getting close to finish!

⚠" It appears that the background/foreground of the items does not follow inherited theme brushes. I would expect the foreground/background to track with whatever was set on the parent window."
Background Fixed. Now I'm fighting (without success) with Foreground / TextElement.Foreground. It doesn't inherit the brush over VisualTree. Default TabControl defines the Background property to White, so, the Background={x:Null} removed from last review comes back.

⚠"There appears to be a rendering issue with the shadows/separator."
Should I remove the shadow? Or just on sample app? (by default it's Depth 0). Ocurred just on dark theme.

✅" The non-rail styles should get pulled out. For the initial release it should only be the navigation rail bits."
If you don't mind, I'll keep the XAML styles commented for a future implementation.

@Keboo
Copy link
Member

Keboo commented Sep 12, 2020

I push some changes that should address the colors (I also added a simple UI test for it).

Regarding the shadow I would suggest that we simply remove it for now. The spec only appears to have the separator so I think it is fine to start small.

I am fine if you simply want to comment out the other tab styles.

@HClausing
Copy link
Contributor Author

HClausing commented Sep 13, 2020

I push some changes that should address the colors (I also added a simple UI test for it).

Regarding the shadow I would suggest that we simply remove it for now. The spec only appears to have the separator so I think it is fine to start small.

I am fine if you simply want to comment out the other tab styles.

Hello @Keboo !

I've seen that you removed this Line from Style:
Setter Property="Background" Value="{x:Null}"

Didn't you have any problems on your tests without this? For me, without this, the default TabControl's Style applies a white solid brush. This is the reason why I insisted setting it to null.

@HClausing
Copy link
Contributor Author

HClausing commented Sep 14, 2020

Next stage of study/work/test:

  • Add a NavigationRailAssit with those attached properties:

  • TextTabMode : Persistent / OnSelect;

  • TextTab : Some short text for each TabItem;

  • BadgeContent : For dynamic TabItem information.

  • Work on NavigationRail template to use new Attached Properties.

@Keboo Keboo merged commit cde5d53 into MaterialDesignInXAML:master Sep 18, 2020
@Keboo
Copy link
Member

Keboo commented Sep 18, 2020

@HClausing I got this merged for the 3.2.0 release. I do have a copy of the work prior to tonight's changes sitting in one of my branches as well.

@Keboo Keboo added this to the 3.2.0 milestone Sep 18, 2020
@Keboo Keboo added demo app Items that relate to the demo application enhancement release notes Items are likely to be highlighted in the release notes. labels Sep 18, 2020
@HClausing
Copy link
Contributor Author

HClausing commented Sep 18, 2020

Hello!

Thanks so much. One doubt: To use the NavigationRail style, it wont' be on Defautls.xaml (like Textbox, etc) and needs to be referenced on merged dictionaries of app.xaml (like DataGrid, PopupBox, etc) ?

@HClausing
Copy link
Contributor Author

HClausing commented Sep 18, 2020

Another thing: The Navigation Rail page on DemoApp appears to be outdated (using de old version with shadows):

image

Last change :

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
demo app Items that relate to the demo application enhancement release notes Items are likely to be highlighted in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants