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 hiding the icon when it's set to "none" #18030

Merged
merged 2 commits into from
Oct 11, 2024
Merged

Conversation

carlos-zamora
Copy link
Member

The settings UI and settings model allow you to set the icon to "none" to hide the icon (you can actually see this effect in the settings UI when changing the value of the profile icon). However, during settings validation, "none" is considered a file path, which is then failed to be parsed, resulting in the icon being marked as invalid and immediately clearing the value.

This PR fixes this issue by considering "none" to be an accepted value during validation.

Related to #15843
Closes #17943

Validation Steps Performed

When an icon is set to "none", ...
✅ no more warning
✅ the icon is hidden

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Oct 10, 2024
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
@carlos-zamora carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Oct 10, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Second It's a PR that needs another sign-off label Oct 11, 2024
@DHowett DHowett merged commit 36f064c into main Oct 11, 2024
20 checks passed
@DHowett DHowett deleted the dev/cazamor/icon-none branch October 11, 2024 00:11
DHowett pushed a commit that referenced this pull request Oct 17, 2024
The settings UI and settings model allow you to set the icon to "none"
to hide the icon (you can actually see this effect in the settings UI
when changing the value of the profile icon). However, during settings
validation, "none" is considered a file path, which is then failed to be
parsed, resulting in the icon being marked as invalid and immediately
clearing the value.

This PR fixes this issue by considering "none" to be an accepted value
during validation.

Related to #15843
Closes #17943

## Validation Steps Performed
When an icon is set to "none", ...
✅ no more warning
✅ the icon is hidden

(cherry picked from commit 36f064c)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgTyV8A
Service-Version: 1.21
DHowett pushed a commit that referenced this pull request Oct 17, 2024
The settings UI and settings model allow you to set the icon to "none"
to hide the icon (you can actually see this effect in the settings UI
when changing the value of the profile icon). However, during settings
validation, "none" is considered a file path, which is then failed to be
parsed, resulting in the icon being marked as invalid and immediately
clearing the value.

This PR fixes this issue by considering "none" to be an accepted value
during validation.

Related to #15843
Closes #17943

## Validation Steps Performed
When an icon is set to "none", ...
✅ no more warning
✅ the icon is hidden

(cherry picked from commit 36f064c)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgTyV8E
Service-Version: 1.22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
Development

Successfully merging this pull request may close these issues.

Feature request: option to remove profile icon completely
3 participants