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

Combobox visual updates #3938

Merged
merged 21 commits into from
Feb 1, 2021
Merged

Combobox visual updates #3938

merged 21 commits into from
Feb 1, 2021

Conversation

beervoley
Copy link
Contributor

@beervoley beervoley commented Jan 13, 2021

Description

This update brings acrylic brush, lots of new colors and rounded corners to the control.

I've also added some required updates to TextBox resources and Common_themeresources that are required by the new ComboBox. These changes will be checked in from this PR

What is not done yet

Focus Visuals for the combo box items need to be updated.
Font for editable ComboBox needs to be updated.
Shadows will be fixed in the OS.
Blue underline for Editable ComboBox needs to be sharper.

Screenshots (if appropriate):

image

@beervoley beervoley added area-ComboBox team-Controls Issue for the Controls team labels Jan 13, 2021
@beervoley beervoley requested review from YuliKl, ranjeshj and teaP January 13, 2021 09:18
@beervoley beervoley self-assigned this Jan 13, 2021
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jan 13, 2021
@beervoley
Copy link
Contributor Author

/azp run

@beervoley beervoley removed the needs-triage Issue needs to be triaged by the area owners label Jan 13, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdtauk
Copy link
Contributor

mdtauk commented Jan 13, 2021

Not sure if these are problems with the image preview, or with the actual controls themselves, but I will point them out.

image

  1. Is the backplate needed here for the Selected Item at rest? It could cause some confusion and visual clutter when you are hovering over items.

  2. I believe the updated ListView guidance is that the Selected Indicator should have a minimum height of 16

  3. The disabled state with the Dark Theme also has a backplate, which adds to the confusion with hover states. This backplate does not show on the Light Theme.

I would say, the Backplate for the ComboBoxItems should probably only be visible on PointerOver and Pressed states. The Selection Indicator is clear enough for the Current item.


There is a bigger question about how you would handle grouped headers in a ComboBox - which is another ask, and would bring it to parity with the FluentUI control.

@beervoley
Copy link
Contributor Author

beervoley commented Jan 14, 2021

Not sure if these are problems with the image preview, or with the actual controls themselves, but I will point them out.

image

  1. Is the backplate needed here for the Selected Item at rest? It could cause some confusion and visual clutter when you are hovering over items.
  2. I believe the updated ListView guidance is that the Selected Indicator should have a minimum height of 16
  3. The disabled state with the Dark Theme also has a backplate, which adds to the confusion with hover states. This backplate does not show on the Light Theme.

I would say, the Backplate for the ComboBoxItems should probably only be visible on PointerOver and Pressed states. The Selection Indicator is clear enough for the Current item.

There is a bigger question about how you would handle grouped headers in a ComboBox - which is another ask, and would bring it to parity with the FluentUI control.

  1. I agree that it might be confusing. I'll see what we can do :)
  2. The Selected Indicator is exactly 16 px in Rest state.
  3. The backplate in Dark mode is due to incorrect disabled brush color value in dark mode. It will be fixed pretty soon :)

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@karkarl
Copy link
Contributor

karkarl commented Jan 29, 2021

        <x:Double x:Key="ComboBoxArrowThemeFontSize">21</x:Double>

A lot of these Double and Thickness resources are duplicated across the themes. These can be moved out of the themes to avoid duplication.


Refers to: dev/ComboBox/ComboBox_themeresources.xaml:13 in f6b4e52. [](commit_id = f6b4e52, deletion_comment = False)

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley beervoley changed the title Combobox visual updates Combobox visual updates - don’t merge Feb 1, 2021
@beervoley beervoley changed the title Combobox visual updates - don’t merge Combobox visual updates Feb 1, 2021
@beervoley beervoley merged commit 6109cfa into master Feb 1, 2021
@beervoley beervoley deleted the vsiliushi/combobox_update branch February 1, 2021 19:10
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Nov 14, 2022
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ComboBox team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants