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

Fix the SmartRename button regressions #75134

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

aoyangyou
Copy link
Contributor

@aoyangyou aoyangyou commented Sep 16, 2024

This PR fix the SmartRename button regressions: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2112161/

  1. Suggestion panel behavior
    When IsAutomaticSuggestionsEnabled is ON
    Button toggled => suggestion panel collapse
    Button not toggled = > suggestion panel expand

2 Button UI when Hover / Focus / toggle / click
Click => lighter border + purple background (indicate IsAutomaticSuggestionsEnabled is ON)
Focus => lighter border
Toggle => lighter border + grey background
Hover => grey background
renamebuttoncolorRemoveBackground3

In blue and light theme
renamebuttoncolorRemoveBackgroundBlueLight

@aoyangyou aoyangyou requested a review from a team as a code owner September 16, 2024 19:48
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 16, 2024
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. Needs UX Triage labels Sep 16, 2024
@@ -87,7 +87,6 @@
</Trigger>
<Trigger Property="IsKeyboardFocused" Value="true">
<Setter Property="BorderBrush" Value="{DynamicResource {x:Static vsui:CommonControlsColors.ButtonBorderFocusedBrushKey}}"/>
<Setter Property="Background" Value="{DynamicResource {x:Static vsui:CommonControlsColors.ButtonFocusedBrushKey}}"/>
Copy link
Member

Choose a reason for hiding this comment

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

Why this is not needed?

Copy link
Contributor Author

@aoyangyou aoyangyou Sep 18, 2024

Choose a reason for hiding this comment

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

@Cosifne Hi, we want to make it consistent with the panel expand case (Purple), and different with the panel collapse case. It decreases the confusion.

After removing the focus background, the button will be purple when focused. And the light border can indicate whether the button is focused.

Copy link
Member

Choose a reason for hiding this comment

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

Could you verify if the color still looks correct when in light/blue theme?

@aoyangyou aoyangyou requested a review from Cosifne September 19, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Needs UX Triage untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants