Manually set the automation name of the default color scheme for screen reader

[PankajBhojwani]

Summary of the Pull Request

When we navigate to the color schemes page, find the default color scheme (if present) and manually set the container's automation name to include the word 'default'.

Note that we don't want to change the actual ColorSchemeViewModel's name since that name is used internally to identify schemes. We only want to change the ListViewItem's name, i.e. the container in the SUI.

PR Checklist

• Closes [Color Schemes]: Screen reader does not announce the 'default' visible text defined with the 'Campbell' color scheme on the page. #14401
• CLA signed. If not, go over here and sign the CLA
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Validation Steps Performed

Screen reader reads out the 'default' text now. It also correctly reads out the new default scheme if the default scheme is changed via SUI or json.

[carlos-zamora]

@msftbot merge this in 10 minutes

[ghost]

Hello @carlos-zamora!

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 won't merge this pull request until after the UTC date Thu, 19 Jan 2023 19:38:47 GMT, which is in 10 minutes

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".

[DHowett]

I'm sorry, won't this only work for the default that is set when the page is first navigated to? Will it happen again when you navigate back to the page after changing the default?

Even if it does, it never fixes the name of the non-default color schemes when you navigate back to it... so two or more schemes will be named "xxx default" when you change them.

Is it even a guarantee that the list item container is the same in a virtualized list?

I don't think this should have merged...

[DHowett]

Unless we're reconstructing the entire scheme list every time we navigate to, or back to, that page. I don't believe we are doing so...?

[DHowett]

Also, in general, we should try to avoid doing things in "layout updated" handlers. That's a pattern that we used for a very specific reason back in TermControl when it didn't have another event to notify when its size was finalized. This should, at least, have a BODGY or WORKAROUND comment.

[PankajBhojwani]

Unless we're reconstructing the entire scheme list every time we navigate to, or back to, that page. I don't believe we are doing so...?

That does seem to be what's happening! At first I thought we would need to 'un-name' the old default scheme's container when a new scheme gets selected as default, but it turns out that when we navigate back to the ColorSchemes page (or the UI refreshes itself because of a change in the json) the old containers don't exist. So we just need to set the new container's name.

Also, in general, we should try to avoid doing things in "layout updated" handlers.

Is there a different handler we should use? Because inside OnNavigatedTo the ListView reads as empty (which makes sense since that is exactly where we set the underlying ViewModel, so the View hasn't been populated yet).

[DHowett]

I don't think this should have merged...

I was too hasty in saying this, and it was not very nice. I'm sorry about that 😻

[ghost]

🎉Windows Terminal Preview v1.17.1023 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download