Skip to content

Conversation

@niels9001
Copy link
Collaborator

@niels9001 niels9001 commented Feb 24, 2023

See #392

This PR introduces the following changes:

  • Adds the AutoSelection property (by default the first item will be selected - this can be disabled if needed)
    AutoSelection

  • High contrast improvements
    SegmentedControlHCAfter

@niels9001 niels9001 marked this pull request as draft February 24, 2023 15:49
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Ugh, silly GitHub requires a comment here if you do things out of order... 😋

@michael-hawker
Copy link
Member

michael-hawker commented Feb 25, 2023

@niels9001 FYI, I noticed you had some usages of as with pragmas to ignore warnings of the null deref. Easier/safer pattern is to use the is typing like so:

        if (element is SegmentedItem segmentedItem)
        {
            segmentedItem.Loaded += SegmentedItem_Loaded;
        }

@michael-hawker
Copy link
Member

michael-hawker commented Feb 25, 2023

FYI @niels9001 I noticed at different DPI (150?) the padding (gaps) is off:

image

image

image

@michael-hawker
Copy link
Member

michael-hawker commented Feb 25, 2023

Same C# is cleanup can be done here:

            // Only do stuff if the index is actually changing
            if (index != previousIndex && ContainerFromIndex(index) is SegmentedItem newItem)
            {
                newItem.Focus(FocusState.Keyboard);
                retVal = true;
            }
        }

        return retVal;
    }

    private SegmentedItem? GetCurrentContainerItem()
    {
        if (ControlHelpers.IsXamlRootAvailable && XamlRoot != null)
        {
            return FocusManager.GetFocusedElement(XamlRoot) as SegmentedItem;
        }
        else
        {
            return FocusManager.GetFocusedElement() as SegmentedItem;
        }
    }

@michael-hawker
Copy link
Member

michael-hawker commented Feb 25, 2023

@niels9001 was looking into the selection issue with SelectedIndex. I think the issue is that SelectedIndex/Item is read first before the Items collection has loaded, so there's a timing issue. (Also, through XAML the items load one-by-one I think.)

Was investigating if we could simplify the logic here than trying to check everytime an item is loaded if we've set the index. Will think about it more Monday.

(Though I'm also still surprised we have to do anything special here. I mean ListViewBase should just be handling all this logic for us.)

@niels9001 niels9001 requested a review from michael-hawker March 7, 2023 15:03
@niels9001 niels9001 marked this pull request as ready for review March 7, 2023 15:04
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Tested on UWP and WinAppSDK, looks great.

@michael-hawker michael-hawker added the experiment 🧪 Used to track issues that are experiments (or their linked discussions) label Mar 7, 2023
@michael-hawker
Copy link
Member

@Arlodotexe feel free to take a quick poke if you'd like and/or just merge.

@michael-hawker michael-hawker linked an issue Mar 8, 2023 that may be closed by this pull request
19 tasks
@Arlodotexe Arlodotexe merged commit ce74acf into main Mar 8, 2023
@delete-merged-branch delete-merged-branch bot deleted the niels9001/segmented-hc branch March 8, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experiment 🧪 Used to track issues that are experiments (or their linked discussions)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🧪 [Experiment] Segmented

3 participants