Retain (and indicate) orphaned dynamic profiles #18188
Conversation
microsoft-github-policy-service
bot
added
Issue-Bug
|
@zadjii-msft does this actually close #9997? |
| @@ -621,13 +630,33 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |||
| _Navigate(profileViewModel, BreadcrumbSubPage::None); | |||
| } | |||
|
|
|||
| static MUX::Controls::InfoBadge _createGlyphIconBadge(wil::zwstring_view glyph) | |||
| { | |||
| MUX::Controls::InfoBadge badge; | |||
There was a problem hiding this comment.
FWIW apparently you can give resources an x:Key look them up in the resource dictionary in the code side and then set them as the style programmatically.
There was a problem hiding this comment.
Yes and no. AFAIK It still doesn't work for Theme Dictionaries - you need to look it up yourself in the final leaf node dictionary.
|
|
||
| <local:SettingContainer x:Uid="Profile_Name"> | ||
| <local:SettingContainer.Content> | ||
| <TextBlock FontFamily="Segoe UI, Segoe Fluent Icons, Segoe MDL2 Assets" |
There was a problem hiding this comment.
Meh, it's a convention we've used for all of our text fields - I suspect it's carried over from Icon, where we want to display MDL2 icons
There was a problem hiding this comment.
But there's no free lunch. DirectWrite requires the creation of a custom font fallback collection for this to work, which is quite expensive. I suspect we cannot expect WinUI to be capable enough to cache such frequently used collection.
As such, I'd rather say we should change all the places that use them without having a reason to.
There was a problem hiding this comment.
I will follow up with this then!
| if (profile.Orphaned()) | ||
| { | ||
| profileNavItem.InfoBadge(_createGlyphIconBadge(L"\xE7BA") /* Warning Triangle */); | ||
| } | ||
| else if (profile.Hidden()) | ||
| { | ||
| profileNavItem.InfoBadge(_createGlyphIconBadge(L"\xED1A") /* Hide */); | ||
| } |
There was a problem hiding this comment.
Can we add a tooltip and AutomationProperties.HelpText to profileNavItem to explain what these icons mean to the user? Screen readers won't even know the info badge is there!
There was a problem hiding this comment.
WinUI already uses the tooltip for the profile name when it's too long. I can add a tooltip on the badge itself, and automation text, but narrator doesn't read it. Why is our UI framework bad?
|
Removed from body It looks like this:
If you disable all profile sources, it looks barren indeed:
|
The original intent with dynamic profiles was that they could be uninstalled but that Terminal would remember your settings in case they ever came back. After we implemented dynamic profile _deletion_, however, we accidentally made it so that saving your settings after a dynamic profile disappeared scoured it from the planet _forever_ (since we remembered that we generated it, but now it was no longer in the settings file). This pull request implements: - Tracking for orphaned dynamic profiles Closes #14061 Closes #11510 Refs #13916 Refs #9997 Modified for 1.22. I am not including any of the UI affordances. (cherry picked from commit 90866c7) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgU1-p4 Service-Version: 1.22
The original intent with dynamic profiles was that they could be uninstalled but that Terminal would remember your settings in case they ever came back.
After we implemented dynamic profile deletion, however, we accidentally made it so that saving your settings after a dynamic profile disappeared scoured it from the planet forever (since we remembered that we generated it, but now it was no longer in the settings file).
This pull request implements:
Closes #14061
Closes #11510
Refs #13916
Refs #9997
It looks like this:
If you disable all profile sources, it looks barren indeed: