Skip to content

Conversation

@jebriede
Copy link
Contributor

Fixed the style of the Dependencies tree's root nodes so the roots appear correctly when selected in all themes. Also fixed the dashed border color of the Dependencies TreeView control for all themes.

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/715
Regression: No

  • Last working version: N/A
  • How are we preventing it in future: N/A

Fix

Details: The Dependencies TreeView was setting an explicit foreground color which was overriding the color change when the node was selected. Removed the explicit color setting and added a style that applies to all root nodes that gives it the right color. Also applied the FocusVisualStyle for all themed controls so the dashed border would look correct in the dark theme as well as all others.

Testing/Validation

Tests Added: No
Reason for not adding tests: Fix is purely visual in the UI
Validation: For each theme, tabbed into the Dependencies tree view, observed the border was the appropriate color for the theme, then pressed the Down arrow key which selects the root node of the tree, and using Accessibility Insights, validated that the color ratio of the text to background was greater than 4.5:1.

@jebriede jebriede requested a review from a team as a code owner January 15, 2021 03:31
Copy link
Contributor

@donnie-msft donnie-msft left a comment

Choose a reason for hiding this comment

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

Nice! Any reason not to set that style on all TreeViews instead of just this one?

…orrectly when selected in all themes. Also fixed the border color of the Dependencies TreeView control for all themes.
@jebriede jebriede force-pushed the dev-jebriede-FixDependenciesColorsPMUI branch from 5d08c2f to b5d831a Compare January 15, 2021 23:41
@jebriede jebriede merged commit f308eaa into dev Jan 16, 2021
@jebriede jebriede deleted the dev-jebriede-FixDependenciesColorsPMUI branch January 16, 2021 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants