[Feature] Add TabbedCommandBar (ribbon) control #3551

yoshiask · 2020-11-02T21:59:39Z

Moved to #3556

Fixes

#3259

PR Type

What kind of change does this PR introduce?

Feature

Sample app changes

What is the current behavior?

There is no ribbon or ribbon-like control for UWP. The current solution is to put CommandBars inside of a Pivot.

What is the new behavior?

Adds a TabbedCommandBar, which internally is almost identical to the current solutions, but provides a much friendlier developer experience.

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Pull Request has been submitted to the documentation repository instructions. Link: Create TabbedCommandBar.md MicrosoftDocs/WindowsCommunityToolkitDocs#405
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
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

ghost · 2020-11-02T21:59:42Z

Thanks yoshiask for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

net-foundation-cla · 2020-11-02T21:59:53Z

All CLA requirements met.

mdtauk · 2020-11-03T01:10:23Z

I am very happy with the name TabbedCommandBar - I would be happy if this control and name could make it's way to WinUI proper in the future.

…ocs don't exist yet)

Started writing the documentation for [TabbedCommandBar](CommunityToolkit/WindowsCommunityToolkit#3551)

michael-hawker

Did a quick review, will need to try out a bit more, but some general feedback on a couple of things to clean-up (like massive commented xaml sections?).

For the sample app control image, I was wondering if there should be the three white dots that represent the commandbar in the second row:

michael-hawker · 2020-11-03T17:15:53Z

Windows Community Toolkit.sln

 		{42CA4935-54BE-42EA-AC19-992378C08DE6}.Release|x64.Build.0 = Release|Any CPU
 		{42CA4935-54BE-42EA-AC19-992378C08DE6}.Release|x86.ActiveCfg = Release|Any CPU
 		{42CA4935-54BE-42EA-AC19-992378C08DE6}.Release|x86.Build.0 = Release|Any CPU
-		{A5E98964-45B1-442D-A07A-298A3221D81E}.Debug|Any CPU.ActiveCfg = Debug|Win32


Any idea what these represent and why there are removed?

michael-hawker · 2020-11-03T17:16:54Z

Microsoft.Toolkit.Uwp.UI.Controls/TabbedCommandBar/TabbedCommandBar.xaml

+                                    <PopInThemeAnimation TargetName="PART_RibbonContent" />
+                                </Storyboard>
+                                <Style TargetType="ComboBox" />
+                                <!--<Style TargetType="ComboBox">


Any reason to keep these big comment blocks?

The reason for this one (the ComboBox style) is because there's currently a bug that causes an unhandled native exception in WUX when the ComboBox style is overridden here. I've yet to make a minimum repo for it. Anyway, that style is supposed to make ComboBoxes look presentable in the primary commands space and also play nicely with overflow (because ATM they're completely unusable once they get put into the overflow menu).

Presumably that style should be placed in its own file and given a key (like ComboBoxAppBarStyle)?

Also, I don't want to make too many changes to the ComboBox style. Is there any way to change parts of the template while leaving other parts of it as the default, or do I have to retemplate the entire control?

@yoshiask not with ComboBox as it doesn't expose it's template as a key we can reference link you could with CommandBarRevealStyle. If it did and you were only adjusting properties, you could just modify the setters without the template....

Agree we should try and stick it in it's own file and then just merge in with the resource dictionary here.

michael-hawker · 2020-11-03T17:17:53Z

Microsoft.Toolkit.Uwp.UI.Controls/TabbedCommandBar/TabbedCommandBar.xaml

+    <!--  For some reason this code causes an exception at runtime, that String couldn't be turned into a Uri.  -->
+    <!--<ResourceDictionary.MergedDictionaries>
+        <ResourceDictionary Source="ms-appx:///Microsoft.Toolkit.Uwp.UI.Controls.Ribbon/TabbedCommandBar/TabbedCommandBar.xaml" />
+        <ResourceDictionary Source="ms-appx:///Microsoft.Toolkit.Uwp.UI.Controls.Ribbon/TabbedCommandBar/TabbedCommandBarItem.xaml" />


We should have the Item style in it's own XAML file and merge it in here (see what we did for the Tokenizing Text Box).

michael-hawker · 2020-11-03T17:18:53Z

Microsoft.Toolkit.Uwp.UI.Controls/Microsoft.Toolkit.Uwp.UI.Controls.csproj

+     <Page Update="Ribbon\Ribbon.xaml">
+       <Generator>MSBuild:Compile</Generator>
+     </Page>


Suggested change

<Page Update="Ribbon\Ribbon.xaml">

<Generator>MSBuild:Compile</Generator>

</Page>

This looks extra?

michael-hawker · 2020-11-03T17:20:57Z

Microsoft.Toolkit.Uwp.UI.Controls.Design/TabbedCommandBarMetadata.cs

+using Microsoft.Windows.Design.PropertyEditing;
+using System.ComponentModel;
+
+namespace Microsoft.Toolkit.Uwp.UI.Controls.Design


We're moving to the new format in #3318. I don't think there's a way for you to pre-emptively move there though until that's merged.

@Nirmal4G correct?

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/TabbedCommandBar/TabbedCommandBar.bind

ghost · 2020-11-03T19:10:46Z

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

michael-hawker · 2020-11-03T23:14:42Z

Few more comments.

Where is 'Test' coming from??? (I can't find it.)

For the secondary commands:

Can we request the dropdown placement to go down vs. up?

Also, is it easy to provide an option for the '...' to be either right-aligned to the panel like it is now OR to be next to the last item in the list if there's not enough to fill it?

Have a few more inline code comments. It definitely looks like something is holding a reference to the items and not letting them go. I wonder if it has something to do with NavigationView itself as we aren't using NavigationViewItem directly? I was able to bind to MenuItemsSource directly and that exposed the problem a bit more clearly.

michael-hawker

Forgot to submit my comments. 🤦‍♂️

michael-hawker · 2020-11-03T22:25:32Z

Microsoft.Toolkit.Uwp.UI.Controls/TabbedCommandBar/TabbedCommandBar.xaml

+        <ResourceDictionary Source="ms-appx:///Microsoft.Toolkit.Uwp.UI.Controls.Ribbon/TabbedCommandBar/TabbedCommandBarItem.xaml" />
+    </ResourceDictionary.MergedDictionaries>-->
+
+    <Style BasedOn="{StaticResource CommandBarRevealStyle}"


Yeah, I'm going to say we should split this in the file and instead of referencing the template, copy it. As then we can expose the properties for configuring where the 'More' button is aligned, and then we won't be trying to grab them in the OnApplyTemplate to set properties in code-behind.

michael-hawker · 2020-11-03T22:29:00Z

Microsoft.Toolkit.Uwp.UI.Controls/TabbedCommandBar/TabbedCommandBar.xaml

+                                    <PopInThemeAnimation TargetName="PART_RibbonContent" />
+                                </Storyboard>
+                                <Style TargetType="ComboBox" />
+                                <!--<Style TargetType="ComboBox">


@yoshiask not with ComboBox as it doesn't expose it's template as a key we can reference link you could with CommandBarRevealStyle. If it did and you were only adjusting properties, you could just modify the setters without the template....

Agree we should try and stick it in it's own file and then just merge in with the resource dictionary here.

michael-hawker · 2020-11-03T22:58:35Z

Microsoft.Toolkit.Uwp.UI.Controls/TabbedCommandBar/TabbedCommandBar.xaml

+                                    </NavigationViewItem.Resources>
+                                </NavigationViewItem>
+                            </DataTemplate>
+                            <controls:TabbedCommandBarItemTemplateSelector x:Key="DefaultTabbedCommandBarItemTemplateSelector"


I feel like we want to be using a container style selector vs a template one? Though I didn't see it duplicating NavigationViewItems so that was good. Not sure how easy it'd be to do the resource overrides like you're doing with a style selector vs. a template selector.

We should probably expose all these outside the Grid so that developers can override them, it's just going to be easier to expose them as Styles vs. DataTemplates for what developers are used to. (Like they'd probably expect them as style properties on the TabbedCommandBar to be able to override with their own style.)

michael-hawker · 2020-11-03T23:11:02Z

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/TabbedCommandBar/TabbedCommandBar.bind

+            <controls:TabbedCommandBar.Resources>
+                <StaticResource x:Key="NavigationViewTopPaneBackground" ResourceKey="SystemControlChromeMediumLowAcrylicWindowMediumBrush" />
+                <StaticResource x:Key="TabbedCommandBarContentBackground" ResourceKey="SystemControlChromeLowAcrylicWindowBrush" />
+            </controls:TabbedCommandBar.Resources>


I didn't notice a big difference without these resources? We should just have the example be as default as possible.

michael-hawker · 2020-11-06T19:13:24Z

Closing this now that #3556 is open.

yoshiask · 2020-11-06T19:13:53Z

Dang that was quick 😆

## Fixes #3259   ## PR Type What kind of change does this PR introduce?   Feature     Sample app changes  ## What is the current behavior? There is no ribbon or ribbon-like control for UWP. The current solution is to put CommandBars inside of a Pivot. ## What is the new behavior? Adds a TabbedCommandBar, which internally is almost identical to the current solutions, but provides a much friendlier developer experience. ## PR Checklist Please check if your PR fulfills the following requirements: - [ ] Tested code with current [supported SDKs](../readme.md#supported) - [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: MicrosoftDocs/WindowsCommunityToolkitDocs#405 - [ ] Sample in sample app has been added / updated (for bug fixes / features) - [x] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets) - [ ] 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*) - [x] Contains **NO** breaking changes  ## Other information I forgot to do my work on a feature branch, so this PR is basically a duplicate of #3551, with a little bit of cleanup.

Started writing the documentation for [TabbedCommandBar](CommunityToolkit/WindowsCommunityToolkit#3551)

Add initial work for TabbedCommandBar

255eee8

ghost requested review from Kyaa-dost, azchohfi and michael-hawker November 2, 2020 21:59

yoshiask added 2 commits November 2, 2020 16:05

Add very minimal sample [WIP]

6a5399a

Update samples.json

b79e75e

Add thumbnail for TabbedCommandBar and update doc link (even though d…

9303c40

…ocs don't exist yet)

yoshiask added a commit to yoshiask/WindowsCommunityToolkitDocs that referenced this pull request Nov 3, 2020

Create TabbedCommandBar.md

51bb022

Started writing the documentation for [TabbedCommandBar](CommunityToolkit/WindowsCommunityToolkit#3551)

michael-hawker requested changes Nov 3, 2020

View reviewed changes

ghost added the needs author feedback 📝 label Nov 3, 2020

Added slightly mroe detailed sample

4b96f43

ghost added needs attention 👋 and removed needs author feedback 📝 labels Nov 3, 2020

ghost removed the needs attention 👋 label Nov 3, 2020

michael-hawker reviewed Nov 4, 2020

View reviewed changes

Merge branch 'master' into master

a21cd81

yoshiask mentioned this pull request Nov 6, 2020

[Feature] Add TabbedCommandBar (ribbon) control #3556

Merged

7 tasks

michael-hawker closed this Nov 6, 2020

michael-hawker pushed a commit to michael-hawker/WindowsCommunityToolkitDocs that referenced this pull request Mar 9, 2021

Create TabbedCommandBar.md

fd64215

Started writing the documentation for [TabbedCommandBar](CommunityToolkit/WindowsCommunityToolkit#3551)

	<Page Update="Ribbon\Ribbon.xaml">
	<Generator>MSBuild:Compile</Generator>
	</Page>

[Feature] Add TabbedCommandBar (ribbon) control #3551

[Feature] Add TabbedCommandBar (ribbon) control #3551

Uh oh!

Conversation

yoshiask commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Moved to #3556

Fixes

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

Uh oh!

ghost commented Nov 2, 2020

Uh oh!

net-foundation-cla bot commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdtauk commented Nov 3, 2020

Uh oh!

michael-hawker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost commented Nov 3, 2020

Uh oh!

michael-hawker commented Nov 3, 2020

Uh oh!

michael-hawker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michael-hawker commented Nov 6, 2020

Uh oh!

yoshiask commented Nov 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yoshiask commented Nov 2, 2020 •

edited

Loading

net-foundation-cla bot commented Nov 2, 2020 •

edited

Loading