Order enum settings according to enum values #8784

carlos-zamora · 2021-01-13T22:33:39Z

Sorts the list of EnumEntrys that is used to create combo boxes and
radio buttons in the Settings UI. INITIALIZE_BINDABLE_ENUM_SETTING
sorts the list in increasing order of the enum values, whereas
INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER does so in decreasing
order.

References

#6800 - Settings UI Epic

I attempted sorting the IObservableVector<EnumEntry> using
std::sort, but I would get an error in
winrt::Windows::Foundation::swap (C2665). So instead, I did the
following approach:

(unchanged) we're converting the IMap from EnumMappings into (1) a
map of localized strings and (2) the list for XAML controls
instead of storing EnumEntrys to the IObservableVector directly,
store it to a std::vector
sort the vector using std::sort
now initialize the IObservableVector using the sorted
std::vector

This uses the value of the associated enum to determine a sorting order.
Since we want the "negative" value (i.e. "none" or "hidden") to be last,
I use EnumEntryComparator and EnumEntryReverseComparator to
determine whether we want increasing or decreasing order respectively.
INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER is a copy of
INITIALIZE_BINDABLE_ENUM_SETTING, except it uses
EnumEntryReverseComparator to sort in decreasing order.

Closes #8758

carlos-zamora · 2021-01-13T22:35:06Z

@cinnamon-msft Are all of these in the order you want? Or should I reorder any of them? I could introduce custom ordering, if you want.

zadjii-msft · 2021-01-13T22:41:59Z

@msftbot make sure @DHowett signs off on this

ghost · 2021-01-13T22:42:04Z

Hello @zadjii-msft!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I'll only merge this pull request if it's approved by @DHowett

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

zadjii-msft · 2021-01-13T22:42:58Z

I'm tagging Dustin here, for the issue we discussed about sorting IObservableVectors. We really shouldn't need to stash a copy of the vector...

DHowett · 2021-01-13T22:51:22Z

I don't mind this that much. Fortunately we're not stashing a copy. The vector is created, sorted, and then moved into the new instance of IObservable.... It's just the same as what we were doing, but with a sort in the middle. 😄

zadjii-msft · 2021-01-14T11:46:37Z

src/cascadia/TerminalSettingsEditor/Utils.h

+    std::sort(begin(name##List), end(name##List), EnumEntryComparator<enumType>());                                                                     \
+    _##name##List = winrt::single_threaded_observable_vector<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry>(std::move(name##List));
+
+#define INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER(name, enumMappingsName, enumType, resourceSectionAndType, resourceProperty)                      \


I'm sure that we could refactor these macros so there was one base macro that could sort either direction, and then INITIALIZE_BINDABLE_ENUM_SETTING and INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER would just pass the correct arg to the base macro, but ¯\_(ツ)_/¯

ghost · 2021-01-14T11:47:07Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

Sorts the list of `EnumEntry`s that is used to create combo boxes and radio buttons in the Settings UI. `INITIALIZE_BINDABLE_ENUM_SETTING` sorts the list in increasing order of the enum values, whereas `INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER` does so in decreasing order. ## References microsoft#6800 - Settings UI Epic I attempted sorting the `IObservableVector<EnumEntry>` using `std::sort`, but I would get an error in `winrt::Windows::Foundation::swap` (`C2665`). So instead, I did the following approach: - (unchanged) we're converting the `IMap` from EnumMappings into (1) a map of localized strings and (2) the list for XAML controls - instead of storing `EnumEntry`s to the `IObservableVector` directly, store it to a `std::vector` - sort the vector using `std::sort` - _now_ initialize the `IObservableVector` using the sorted `std::vector` This uses the value of the associated enum to determine a sorting order. Since we want the "negative" value (i.e. "none" or "hidden") to be last, I use `EnumEntryComparator` and `EnumEntryReverseComparator` to determine whether we want increasing or decreasing order respectively. `INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER` is a copy of `INITIALIZE_BINDABLE_ENUM_SETTING`, except it uses `EnumEntryReverseComparator` to sort in decreasing order. Closes microsoft#8758

Order enum settings according to enum values

863b1e5

ghost added Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Jan 13, 2021

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 13, 2021

DHowett removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 13, 2021

DHowett approved these changes Jan 14, 2021

View reviewed changes

zadjii-msft approved these changes Jan 14, 2021

View reviewed changes

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 14, 2021

ghost merged commit 9b1bb13 into main Jan 14, 2021

ghost deleted the dev/cazamor/sui/enum-ordering branch January 14, 2021 11:47

carlos-zamora mentioned this pull request Jan 21, 2021

Epic: Settings UI Completion #6800

Open

25 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Order enum settings according to enum values #8784

Order enum settings according to enum values #8784

carlos-zamora commented Jan 13, 2021 •

edited by DHowett

carlos-zamora commented Jan 13, 2021 •

edited by DHowett

zadjii-msft commented Jan 13, 2021

ghost commented Jan 13, 2021

zadjii-msft commented Jan 13, 2021

DHowett commented Jan 13, 2021

zadjii-msft Jan 14, 2021

ghost commented Jan 14, 2021

Order enum settings according to enum values #8784

Order enum settings according to enum values #8784

Conversation

carlos-zamora commented Jan 13, 2021 • edited by DHowett

References

carlos-zamora commented Jan 13, 2021 • edited by DHowett

zadjii-msft commented Jan 13, 2021

ghost commented Jan 13, 2021

zadjii-msft commented Jan 13, 2021

DHowett commented Jan 13, 2021

zadjii-msft Jan 14, 2021

Choose a reason for hiding this comment

ghost commented Jan 14, 2021

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 (@msftbot) and give me an instruction to get started! Learn more here.

carlos-zamora commented Jan 13, 2021 •

edited by DHowett

carlos-zamora commented Jan 13, 2021 •

edited by DHowett

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 (`@msftbot`) and give me an instruction to get started! Learn more here.