Setting PivotItem.Visiblity to Collapsed Does not Remove It from Pivot #2952
Labels
area-Pivot
bug
Something isn't working
needs-triage
Issue needs to be triaged by the area owners
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
msft-github-bot
added
the
needs-triage
52 tasks
ranjeshj
added
needs-winui-3
StephenLPeters
added
area-Pivot
bug
ghost
pushed a commit
to CommunityToolkit/WindowsCommunityToolkit
that referenced
this issue
Nov 9, 2020
# Add ColorPickerButton to the Toolkit Closes #3363 ## PR Type What kind of change does this PR introduce? Feature Documentation content changes Sample app changes ## What is the current behavior? The toolkit, and WinUI/UWP, has no flyout ColorPicker. The color picker that currently exists is a large canvas-type control (that is incorrectly named). This existing control has lots of usability concerns discussed in #3363 and the WinUI repository. ## What is the new behavior? A brand-new control is added to allow selecting and editing a color within a flyout. The selected color is always visible in the content area of a drop-down button as a preview.  The new properties for this picker are below. Note that these are purposely name 'CustomPalette...' instead of 'Palette...' as multiple palettes may be supported in the future (windows colors, recent colors, custom palette all in different sections visible at the same time). ```csharp /// <summary> /// Gets the list of custom palette colors. /// </summary> public ObservableCollection<Windows.UI.Color> CustomPaletteColors { get; } /// <summary> /// Gets or sets the number of colors in each row (section) of the custom color palette. /// A section is the number of columns within an entire row in the palette. /// Within a standard palette, rows are shades and columns are unique colors. /// </summary> public int CustomPaletteColumns { get; set; } /// <summary> /// Gets or sets a custom IColorPalette . /// This will automatically set <see cref="CustomPaletteColors"/> and <see cref="CustomPaletteSectionCount"/> /// overwriting any existing values. /// </summary> public IColorPalette CustomPalette { get; set; } /// <summary> /// Gets or sets a value indicating whether the color palette is visible. /// </summary> public bool IsColorPaletteVisible { get; set; } ``` Making all of this easier is a new IColorPalette interface. Any class can implement this interface to provide a custom color palette such as windows colors (the default), RYB colors, Flat UI colors, etc. Colors could also be algorithmically generated. ```csharp /// <summary> /// Interface to define a color palette. /// </summary> public interface IColorPalette { /// <summary> /// Gets the total number of colors in this palette. /// A color is not necessarily a single value and may be composed of several shades. /// </summary> int ColorCount { get; } /// <summary> /// Gets the total number of shades for each color in this palette. /// Shades are usually a variation of the color lightening or darkening it. /// </summary> int ShadeCount { get; } /// <summary> /// Gets a color in the palette by index. /// </summary> /// <param name="colorIndex">The index of the color in the palette. /// The index must be between zero and <see cref="ColorCount"/>.</param> /// <param name="shadeIndex">The index of the color shade in the palette. /// The index must be between zero and <see cref="ShadeCount"/>.</param> /// <returns>The color at the specified index or an exception.</returns> Color GetColor(int colorIndex, int shadeIndex); } ``` ## PR Checklist Please check if your PR fulfills the following requirements: - [x] Tested code with current [supported SDKs](../readme.md#supported) - [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link --> - [x] 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) - [x] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [x] Contains **NO** breaking changes ## Other information **Control** * [x] Ensure the name ColorPickerButton is accepted * [x] Rename ColorPickerSlider to ColorPickerButtonSlider. ColorPickerSlider is already used by WinUI. * [x] Implement all accessibility functionality. Check tab ordering. * [x] This control originally used NumberBoxes for channel numerical input. Complete porting this functionality over to TextBoxes as the Toolkit does not have a dependency on WinUI. * [x] Support all visual state groups in the original ColorPicker. Connect all functionality * [x] Test changing all ColorPicker properties. Determine which ones make sense to ignore * [x] Trim displayed hex if alpha is not enabled * [x] ~~Switch to radio buttons to switch between RGB/HSV representation. A ComboBox within a Flyout is generally bad practice and is also limited by some bugs in UWP (#1467).~~ * [x] Switch to ToggleButton instead of RadioButton for RGB/HSV representation. Radio buttons don't work in flyouts and controls well due to UWP bugs. ToggleButtons may also visually look better. The design follows a 'segmented control' concept discussed here: microsoft/microsoft-ui-xaml#2310 * [x] Switch the default color palette to be the windows colors (if possible) * [x] Align formatting to the toolkit conventions * [x] Handle CornerRadius in the template. Conditional XAML? Check how other controls do this. * [x] Rename 'CustomPaletteSectionCount' to ~~'CustomPaletteWrapCount'~~ or 'CustomPaletteColumns' like UniformGrid * [x] Update selected palette color data template to provide correct contrast of the border with the selected color ~~to match with windows settings app (check mark overlay)~~ The check mark is not included as the size of swatches in the palette could be smaller than the check mark itself. * [x] Rename 'CustomPaletteColumns' to 'CustomPaletteColumnCount' * [ ] Treat negative/zero numbers in CustomPaletteColumnCount as 'auto' and automatically calculate a good column count to keep rows/columns even. Set the default of this property to 0 'auto'. * [x] Improve spacing above/below channel sliders -- try 24px (2 x 12px) * [ ] Move localizable strings into resources * [x] ~Do not show the color (set the preview border visibility to collapsed) while the control is initializing. This prevents the default color from briefly appearing during initial load. Instead, the background will just show through.~ * [x] Do not change the saturation and value in the hue slider background. Keep SV at maximum. This allows the hue to always be clear to the user and is in line with other implementations (like in Inkscape). * [x] Rename `WindowsColorPalette` to `FluentColorPalette` * [x] Show palette color display name in a tooltip * [x] Use the WinUI algorithm for light/dark color switching * [x] Each tab should have independent visibility using IsColorPaletteVisible (new), IsColorSpectrumVisible (existing) and IsColorChannelTextInputVisible (repurposed to mean the entire tab). The tabs still visible should take up available space remaining. With only one tab visible the tab header should disappear. **Code Review** 1. [x] The mini-palette and color preview will be all one color when the selected color is black or white. The calculated accent colors will not change and pressing them will not change the selected color. * Requires feedback from Microsoft on how the accent colors are calculated in Windows * It is possible to specially handle when the color is as max/min value (black or white) at least the two accent colors to the left/right could be different shades of gray. 1. [x] The mini-palette accent colors are destructive. Pressing back and forward by 1 will not restore the original color. * Again, requires feedback from Microsoft on how the accent colors are calculated in Windows. * Due to perceptual adjustments it's likely this will always be destructive. 1. [x] The third dimension slider in the spectrum tab will vary the other dimensions in the spectrum. This results in the spectrum cursor jumping around as the slider value changes. This might be related to HSV/RGB conversion but needs to be investigated. 1. [x] Adjust the design of the sample app page to move the ColorPickerButton closer to the description. 1. [x] Check if a ColorToDisplayName converter already exists. Consider adding control specific converters to their own sub-namespace. 1. [x] The ColorPickerButton will derive from DropDownButton and have a ColorPicker property. Dependent on #3440 1. [x] The ColorPicker will have the same new style as the ColorPickerButton flyout. While it's a new concept to have tabs in a panel-type control, this is considered acceptable. 1. [x] Merge @michael-hawker branch into this one when given the thumb's up. Both controls will be in the same directory. 1. [x] Remove conditional XAML for corner radius after WinUI 3.0. Conditional XAML doesn't work correctly in resource dictionaries and with setters. See microsoft/microsoft-ui-xaml#2556, fixed with #3440 1. [ ] All slider rendering will be broken out and put into a new ColorPickerSlider. This should simplify code-behind and the default template. 1. [x] The Pivot tab width and show/hide of tabs will be controlled in code-behind. use of the pivot itself is somewhat dependent on issues with touch. The pivot, or switching to a new control, will not hold the release. 1. [ ] Sliders should vary only 1-channel and keep the other channels at maximum. This ensures the gradient always makes sense and never becomes fully black/white/transparent. This could also be made a configuration of the ColorPickerSlider control. Edit: This does need to be configurable as it's only required in HSV mode. 1. [ ] Add color history palette to the sample app **Other** * [x] Complete integration with the sample app. Add XAML to dynamically change the control. * [ ] Complete documentation of the control **Future** These changes should be considered for future versions of the control. They are recorded here for lack of a better location. * [ ] The mini selected color palette may follow the Windows accent color lighter/darker shades algorithm. However, the implementation of this algorithm is currently unknown. This algorithm may be in the XAML fluent theme editor: https://github.com/microsoft/fluent-xaml-theme-editor * [ ] Switch back to NumberBox for all color channel numerical input. This will require WinUI 3.0 and the corresponding release of the toolkit. When microsoft/microsoft-ui-xaml#2757 is implemented, also use NumberBox for hex color input. * [ ] ~Remove the custom 'ColorPickerButtonSlider' after switch to WinUI 3.0. This assumes there is no additional control-specific code at that point. (It currently exists just to work-around UWP bugs).~ This should be turned into it's own full primitive control. * [ ] Update ColorSpectrum_GotFocus event handler (instructions are within code) after/if ColorSpectrum bugs are fixed (likely after WinUI 3.0 * [ ] Use Color.IsEmpty if microsoft/microsoft-ui-xaml#2826 is implemented * [ ] ~Ensure PivotItems are properly removed once/if microsoft/microsoft-ui-xaml#2952 is closed.~
|
I was quite shocked to find out this feature was not implemented in UWP and has been brought up multiple times over the years with no fix. |
|
I get the impression Microsoft sees little value in the Pivot control these days. |
That would be a good question to ask on the next community call. I found comments about this issue going back 6 years with no resolution. I'm considering replacing the pivot control in my app entirely because of this. |
|
The Pivot is not the only control that has been neglected since the beginning. Lack of fixes like this is often cited as one of the reasons devs went back to WPF. On Microsoft's part: since everything historically shared the same UI framework including the OS, breaking changes were nearly impossible. With WinUI 3 decoupled and the compatible version set by each app I hope it all changes and we can finally address things like this. This will be a good litmus test if WinUI 3 will really take off. |
Sergio0694
pushed a commit
to CommunityToolkit/dotnet
that referenced
this issue
Oct 29, 2021
# Add ColorPickerButton to the Toolkit Closes #3363 ## PR Type What kind of change does this PR introduce? Feature Documentation content changes Sample app changes ## What is the current behavior? The toolkit, and WinUI/UWP, has no flyout ColorPicker. The color picker that currently exists is a large canvas-type control (that is incorrectly named). This existing control has lots of usability concerns discussed in #3363 and the WinUI repository. ## What is the new behavior? A brand-new control is added to allow selecting and editing a color within a flyout. The selected color is always visible in the content area of a drop-down button as a preview.  The new properties for this picker are below. Note that these are purposely name 'CustomPalette...' instead of 'Palette...' as multiple palettes may be supported in the future (windows colors, recent colors, custom palette all in different sections visible at the same time). ```csharp /// <summary> /// Gets the list of custom palette colors. /// </summary> public ObservableCollection<Windows.UI.Color> CustomPaletteColors { get; } /// <summary> /// Gets or sets the number of colors in each row (section) of the custom color palette. /// A section is the number of columns within an entire row in the palette. /// Within a standard palette, rows are shades and columns are unique colors. /// </summary> public int CustomPaletteColumns { get; set; } /// <summary> /// Gets or sets a custom IColorPalette . /// This will automatically set <see cref="CustomPaletteColors"/> and <see cref="CustomPaletteSectionCount"/> /// overwriting any existing values. /// </summary> public IColorPalette CustomPalette { get; set; } /// <summary> /// Gets or sets a value indicating whether the color palette is visible. /// </summary> public bool IsColorPaletteVisible { get; set; } ``` Making all of this easier is a new IColorPalette interface. Any class can implement this interface to provide a custom color palette such as windows colors (the default), RYB colors, Flat UI colors, etc. Colors could also be algorithmically generated. ```csharp /// <summary> /// Interface to define a color palette. /// </summary> public interface IColorPalette { /// <summary> /// Gets the total number of colors in this palette. /// A color is not necessarily a single value and may be composed of several shades. /// </summary> int ColorCount { get; } /// <summary> /// Gets the total number of shades for each color in this palette. /// Shades are usually a variation of the color lightening or darkening it. /// </summary> int ShadeCount { get; } /// <summary> /// Gets a color in the palette by index. /// </summary> /// <param name="colorIndex">The index of the color in the palette. /// The index must be between zero and <see cref="ColorCount"/>.</param> /// <param name="shadeIndex">The index of the color shade in the palette. /// The index must be between zero and <see cref="ShadeCount"/>.</param> /// <returns>The color at the specified index or an exception.</returns> Color GetColor(int colorIndex, int shadeIndex); } ``` ## PR Checklist Please check if your PR fulfills the following requirements: - [x] Tested code with current [supported SDKs](../readme.md#supported) - [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link --> - [x] 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) - [x] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [x] Contains **NO** breaking changes ## Other information **Control** * [x] Ensure the name ColorPickerButton is accepted * [x] Rename ColorPickerSlider to ColorPickerButtonSlider. ColorPickerSlider is already used by WinUI. * [x] Implement all accessibility functionality. Check tab ordering. * [x] This control originally used NumberBoxes for channel numerical input. Complete porting this functionality over to TextBoxes as the Toolkit does not have a dependency on WinUI. * [x] Support all visual state groups in the original ColorPicker. Connect all functionality * [x] Test changing all ColorPicker properties. Determine which ones make sense to ignore * [x] Trim displayed hex if alpha is not enabled * [x] ~~Switch to radio buttons to switch between RGB/HSV representation. A ComboBox within a Flyout is generally bad practice and is also limited by some bugs in UWP (#1467).~~ * [x] Switch to ToggleButton instead of RadioButton for RGB/HSV representation. Radio buttons don't work in flyouts and controls well due to UWP bugs. ToggleButtons may also visually look better. The design follows a 'segmented control' concept discussed here: microsoft/microsoft-ui-xaml#2310 * [x] Switch the default color palette to be the windows colors (if possible) * [x] Align formatting to the toolkit conventions * [x] Handle CornerRadius in the template. Conditional XAML? Check how other controls do this. * [x] Rename 'CustomPaletteSectionCount' to ~~'CustomPaletteWrapCount'~~ or 'CustomPaletteColumns' like UniformGrid * [x] Update selected palette color data template to provide correct contrast of the border with the selected color ~~to match with windows settings app (check mark overlay)~~ The check mark is not included as the size of swatches in the palette could be smaller than the check mark itself. * [x] Rename 'CustomPaletteColumns' to 'CustomPaletteColumnCount' * [ ] Treat negative/zero numbers in CustomPaletteColumnCount as 'auto' and automatically calculate a good column count to keep rows/columns even. Set the default of this property to 0 'auto'. * [x] Improve spacing above/below channel sliders -- try 24px (2 x 12px) * [ ] Move localizable strings into resources * [x] ~Do not show the color (set the preview border visibility to collapsed) while the control is initializing. This prevents the default color from briefly appearing during initial load. Instead, the background will just show through.~ * [x] Do not change the saturation and value in the hue slider background. Keep SV at maximum. This allows the hue to always be clear to the user and is in line with other implementations (like in Inkscape). * [x] Rename `WindowsColorPalette` to `FluentColorPalette` * [x] Show palette color display name in a tooltip * [x] Use the WinUI algorithm for light/dark color switching * [x] Each tab should have independent visibility using IsColorPaletteVisible (new), IsColorSpectrumVisible (existing) and IsColorChannelTextInputVisible (repurposed to mean the entire tab). The tabs still visible should take up available space remaining. With only one tab visible the tab header should disappear. **Code Review** 1. [x] The mini-palette and color preview will be all one color when the selected color is black or white. The calculated accent colors will not change and pressing them will not change the selected color. * Requires feedback from Microsoft on how the accent colors are calculated in Windows * It is possible to specially handle when the color is as max/min value (black or white) at least the two accent colors to the left/right could be different shades of gray. 1. [x] The mini-palette accent colors are destructive. Pressing back and forward by 1 will not restore the original color. * Again, requires feedback from Microsoft on how the accent colors are calculated in Windows. * Due to perceptual adjustments it's likely this will always be destructive. 1. [x] The third dimension slider in the spectrum tab will vary the other dimensions in the spectrum. This results in the spectrum cursor jumping around as the slider value changes. This might be related to HSV/RGB conversion but needs to be investigated. 1. [x] Adjust the design of the sample app page to move the ColorPickerButton closer to the description. 1. [x] Check if a ColorToDisplayName converter already exists. Consider adding control specific converters to their own sub-namespace. 1. [x] The ColorPickerButton will derive from DropDownButton and have a ColorPicker property. Dependent on #3440 1. [x] The ColorPicker will have the same new style as the ColorPickerButton flyout. While it's a new concept to have tabs in a panel-type control, this is considered acceptable. 1. [x] Merge @michael-hawker branch into this one when given the thumb's up. Both controls will be in the same directory. 1. [x] Remove conditional XAML for corner radius after WinUI 3.0. Conditional XAML doesn't work correctly in resource dictionaries and with setters. See microsoft/microsoft-ui-xaml#2556, fixed with #3440 1. [ ] All slider rendering will be broken out and put into a new ColorPickerSlider. This should simplify code-behind and the default template. 1. [x] The Pivot tab width and show/hide of tabs will be controlled in code-behind. use of the pivot itself is somewhat dependent on issues with touch. The pivot, or switching to a new control, will not hold the release. 1. [ ] Sliders should vary only 1-channel and keep the other channels at maximum. This ensures the gradient always makes sense and never becomes fully black/white/transparent. This could also be made a configuration of the ColorPickerSlider control. Edit: This does need to be configurable as it's only required in HSV mode. 1. [ ] Add color history palette to the sample app **Other** * [x] Complete integration with the sample app. Add XAML to dynamically change the control. * [ ] Complete documentation of the control **Future** These changes should be considered for future versions of the control. They are recorded here for lack of a better location. * [ ] The mini selected color palette may follow the Windows accent color lighter/darker shades algorithm. However, the implementation of this algorithm is currently unknown. This algorithm may be in the XAML fluent theme editor: https://github.com/microsoft/fluent-xaml-theme-editor * [ ] Switch back to NumberBox for all color channel numerical input. This will require WinUI 3.0 and the corresponding release of the toolkit. When microsoft/microsoft-ui-xaml#2757 is implemented, also use NumberBox for hex color input. * [ ] ~Remove the custom 'ColorPickerButtonSlider' after switch to WinUI 3.0. This assumes there is no additional control-specific code at that point. (It currently exists just to work-around UWP bugs).~ This should be turned into it's own full primitive control. * [ ] Update ColorSpectrum_GotFocus event handler (instructions are within code) after/if ColorSpectrum bugs are fixed (likely after WinUI 3.0 * [ ] Use Color.IsEmpty if microsoft/microsoft-ui-xaml#2826 is implemented * [ ] ~Ensure PivotItems are properly removed once/if microsoft/microsoft-ui-xaml#2952 is closed.~
|
I doubt Microsoft is going to fix the pivot so someone from the team can close this if they want. Also, if the Pivot gets replaced this can be closed. |
|
@robloo, thank you for opening this issue and additional insight in your comments. Pivot is indeed not recommended currently (NavigationView or TabView are preferred), so indeed this does not currently make the bugfix bar. More information here: (https://learn.microsoft.com/en-us/windows/apps/design/controls/pivot#use-navigationview-instead-of-pivot) |
|
@DmitriyKomin It was widely discussed and even mentioned in the community call some years ago that the NavigationView and TabView are NOT full replacements for Pivot. The WinUI team agreed to find a replacement even if that meant porting Pivot -- which is why Pivot was added back albeit with original code that includes this issue. NavigationView is an App-level navigation control -- clearly not a TabControl (using the classic terminology). You can't seriously consider that a replacement. TabView is closer but was unfortunately designed and over constrained as a document-level/top-level tab control. Not suited to tabbing through lighter content such as settings. Pivot still is the best for a TabControl that we've had since WinForms. |
microsoft-github-policy-service
bot
added
the
needs-triage
WinAppSDK 1.5 has a Pivot replacement called Selector Bar |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Setting PivotItem.Visiblity to Collapsed does not remove a PivotItem, it only collapses the Content.
A common scenario when using a Pivot for tab navigation requires sometimes hiding a tab. This was always possible in other controls like in WPF's TabControl which Pivot was supposed to replace. However, in UWP, setting PivotItem.Visiblity=Collapsed will hide the Content but the PivotItem and its header are still shown within the Pivot and appear in the VisualTree. This means there is no way to hide a pivot's 'tab'.
This is a very old issue that has been reported years ago on user voice (before it was deleted) and is on multiple forums. It's something that should be simple to address in the platform but is complex to work-around in applications.
Steps to reproduce the bug
Expected behavior
If the PivotItem itself is Collapsed the Header and the entire PivotItem should be removed from the VisualTree. This also means keyboard navigation is no longer possible.
Screenshots
Version Info
NuGet package version:
Microsoft.UI.Xaml 2.4.0
Additional context
This will require WinUI 3.0 to be fixed. I will fix it myself if it comes to it -- this has been a thorn for years.
The text was updated successfully, but these errors were encountered: