Bind Profile Color Schemes #8388
Conversation
| const auto& colorSchemeMap{ _State.Schemes() }; | ||
| for (const auto& pair : colorSchemeMap) | ||
| { | ||
| _ColorSchemeList.Append(pair.Key()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
if you navigate back to the same page, isn't this gonna make the color scheme list grow without bound?
There was a problem hiding this comment.
Surprisingly no. And I can't figure out why.
| // winrt::com_arrays prevent data binding. | ||
| // Instead of representing Table as a property, | ||
| // we expose the getter as a function. | ||
| Windows.UI.Color[] Table(); |
There was a problem hiding this comment.
wait but why tho? Are we using Table() anywhere?
There was a problem hiding this comment.
So, the funny thing is, Table { get; }; and Table(); in the IDL are both called with Table() in C++. So this doesn't affect anybody calling it haha.
As for why this is happening, the compiler is asserting that winrt::com_array<winrt::Windows::UI::Color> must be a WinRT type so that it can be wrapped in an IReference. Why a winrt::com_array isn't a WinRT type, idk. So changing it to a function is just easier haha
There was a problem hiding this comment.
Wait but why is the compiler asserting this now, and not before?
There was a problem hiding this comment.
Ah, because of this DataTemplate in Profiles.xaml:
<DataTemplate x:DataType="model:ColorScheme">
<TextBlock Text="{x:Bind Name, Mode=OneWay}"/>
</DataTemplate>Even thought we're only binding Name, XAML tries to make every property bindable. Since Table was a property, we'd hit that error :/
carlos-zamora
added
the
AutoMerge
|
Hello @carlos-zamora! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
Summary of the Pull Request
Binds
Profile.ColorSchemeto the list of Color Schemes available in the Settings UI.References
#1564 - Settings UI
Detailed Description of the Pull Request / Additional comments
To my knowledge, there is no way to bind an
IMapViewtoItemsSource.ItemsSourcerequires anIObservableVector, so we need to manually populateColorSchemeListwhen we navigate to the page.CurrentColorSchemeoperates mostly like a standard getter/setter, except it needs to account for the upcoming scenario when a color scheme is renamed or deleted. For now, we fallback to Campbell if the scheme was not found. I would like to make it update automatically, but I feel that we have two approaches here:Profile.ColorSchemeas aColorSchemeinstead of only the nameProfile.ColorSchemeaccordinglyOpen to discuss these options or any other approaches.
Validation Steps Performed
✅ selected item initialized correctly
✅ list initialized correctly
✅ selecting a new color scheme, then creating a new terminal from that profile uses the new color scheme
✅ setting the color scheme name to a color scheme that does not exist selects "Campbell"