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

Color Picker custom style not load on selected pane #4121

Closed
JanRajnoha opened this issue Jul 20, 2021 · 6 comments · Fixed by #4134
Closed

Color Picker custom style not load on selected pane #4121

JanRajnoha opened this issue Jul 20, 2021 · 6 comments · Fixed by #4134
Assignees
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 controls 🎛️
Milestone

Comments

@JanRajnoha
Copy link

Describe the bug

I want to set color picker style like this to have only predefined pallet with colors
image

Problem is color picker load with spectrum selector, which is wrong. This selector is not even in top menu to select.
image

So when I switch to pallet, color picker will load only on pallet, but it should load like this on first open.
image

Steps to Reproduce

XAML code provided in steps to reproduce

Steps to reproduce the behavior:

  1. Copy code to app
<toolkitControls:ColorPickerButton Height="40" Margin="0 4 0 0" HorizontalAlignment="Stretch" VerticalAlignment="Bottom" RelativePanel.AlignRightWithPanel="True" RelativePanel.AlignLeftWithPanel="True">    
  <toolkitControls:ColorPickerButton.ColorPickerStyle>
      <Style TargetType="toolkitControls:ColorPicker">
       <Setter Property="IsColorSpectrumVisible" Value="False"/>
       <Setter Property="IsAlphaEnabled" Value="False"/>
       <Setter Property="IsAlphaSliderVisible" Value="False"/>
       <Setter Property="IsAlphaTextInputVisible" Value="False"/>
       <Setter Property="IsColorChannelTextInputVisible" Value="False"/>
       <Setter Property="IsColorPreviewVisible" Value="True"/>
       <Setter Property="IsColorSliderVisible" Value="False"/>
       <Setter Property="IsHexInputVisible" Value="False"/>
      </Style>
  </toolkitControls:ColorPickerButton.ColorPickerStyle>
</toolkitControls:ColorPickerButton>
  1. Click on color picker button in running app

Expected behavior

Should open directly on color pallet

Screenshots

In description

Environment

NuGet Package(s):
CommunityToolkit.WinUI.UI.Controls - 7.0.3

Package Version(s):

Windows 11 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] May 2020 Update (19041)
- [x] Insider Build (build number: 22000.71)

App min and target version:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [x] October 2018 Update (17763) - minimum
- [ ] May 2019 Update (18362)
- [x] May 2020 Update (19041) - target
- [ ] Insider Build (xxxxx)

Device form factor:
- [x] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio
- [ ] 2017 (version: )
- [x] 2019 (version: 10.3)
- [x] 2019 Preview (version:  11 preview 3)

Additional context

Add any other context about the problem here.

@JanRajnoha JanRajnoha added the bug 🐛 An unexpected issue that highlights incorrect behavior label Jul 20, 2021
@ghost ghost added the needs triage 🔍 label Jul 20, 2021
@ghost
Copy link

ghost commented Jul 20, 2021

Hello JanRajnoha, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker
Copy link
Member

FYI @robloo

@robloo
Copy link
Contributor

robloo commented Jul 20, 2021

This might also be related to the SwitchPresenter as well. Regardless, it should be an easy fix with some update code added to the Loaded event. I'll take a look soon and see where the problem might be occurring though.

@michael-hawker
Copy link
Member

michael-hawker commented Jul 23, 2021

Thanks @robloo for taking a look! The SwitchPresenter binds to the ColorPanelSelector item.

<controls:SwitchPresenter x:Name="ContentContainer"
Grid.Row="1"
Margin="12"
Value="{Binding ElementName=ColorPanelSelector, Path=SelectedItem.Name}">

So not sure if it's an issue with the SelectedItem being null at first maybe or something else. The IsSelected property is hard-coded in the style:

<ListBoxItem x:Name="SpectrumListBoxItem"
AutomationProperties.Name="Spectrum"
IsSelected="True">
<!-- TODO: Add These via x:Uid -->
<FontIcon Glyph="&#xE76D;" />
</ListBoxItem>

Looks like the Visual State toggles the Visibility here:

<VisualState x:Name="ColorSpectrumCollapsed">
<VisualState.Setters>
<Setter Target="SpectrumListBoxItem.Visibility" Value="Collapsed" />
</VisualState.Setters>
</VisualState>

Therefore, it's invisible, but still selected maybe? As we wouldn't have updated the default selected item to whatever's left... 🤔

If there is an issue with the SwitchPresenter not reacting to an initial binding change though, let me know. That should be fairly well tested though. But I think this is a SelectedItem issue... May make sense to remove the hard-coded IsSelected and in OnApplyTemplate/Loaded check which options are visible in order and select the first one maybe?

@robloo
Copy link
Contributor

robloo commented Jul 23, 2021

May make sense to remove the hard-coded IsSelected and in OnApplyTemplate/Loaded check which options are visible in order and select the first one maybe?

Yes, I was thinking the same. Will take a look this weekend.

@robloo
Copy link
Contributor

robloo commented Jul 26, 2021

@michael-hawker This issue is partially related to the SwitchPresenter. The ColorPicker code does not handle "tab" selection at all. It instead is designed to rely on selection automatically being updated by the container. This means if a "tab" is set to collapsed visibility the container should figure out it can't be selected and then select the next available "tab". Selecting a collapsed tab simply doesn't make sense and I do think the SwitchPresenter needs to be smarter about this.

That said, this code never worked correctly due to microsoft/microsoft-ui-xaml#2952. As Pivot clearly has a bad design in this area, and we were changing to SwitchPresenter, it was left with the issue at the time.

There are two possible solutions for this:

  1. Make SwitchPresenter smarter and don't allow a collapsed tab to be selected. If the selected tab is collapsed, it will change the selection to the next visible tab. Actually its the new ListBox that is managing selection. I really don't prefer separating selection and content controls like this. TabControl was much better for things like this.
  2. Update the ColorPicker to set tab visibility itself

In the interest of time, I'm going to proceed with option 2. However, if the architecture allows it, I highly recommend updating SwitchPresenter.

@robloo robloo mentioned this issue Jul 27, 2021
8 tasks
@ghost ghost added the In-PR 🚀 label Jul 27, 2021
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Aug 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 controls 🎛️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants