Apply AutoProps to TextBox settings in SUI

[carlos-zamora]

We already were setting the automation properties on the expander, however, we were not setting it on the content when an expander was present. This change applies the automation properties to both the expander and the child content (i.e. TextBox).

Closes #13827

[DHowett]

Wait, won't this make the UIA tree confusing? In every case, we will have two objects with the same automation properties (AND TOOLTIPS) in the tree, one of which is outside of and much larger than the other.

It feels like it'll tell the user, "There's a control here that will let you set the title. Expand. There's a control here that will let you set the title. Text box."

[DHowett]

I must admit.. I still don't see how this helps. The TextBox is not a Panel, and so both it and the containing control will receive the same tooltip and the same accessibility description. In addition, the tooltip is also displayed on the screen inside the expander.

None of these things actually seem to be the right place for that tooltip, or the associated automation description.

[carlos-zamora]

I must admit.. I still don't see how this helps. The TextBox is not a Panel, and so both it and the containing control will receive the same tooltip and the same accessibility description. In addition, the tooltip is also displayed on the screen inside the expander.

None of these things actually seem to be the right place for that tooltip, or the associated automation description.

The tooltip is a completely separate thing. I would need to verify it, but I think that's an issue with the expander itself where the tooltip is applied to all the child items by default. See Launch Parameters > "Use system default". All of the launch params have the main tooltip for the expander, but "use system default" gets its own because we applied it.

The help text acts differently though. I added the panel check because for something like "Launch size" that looks like this...

the inner stack panel would get the name and help text applied. This caused a weird behavior as follows:

• focus on expander: read expander name and help text
• tab to child control: read expander name and help text (as control type "grouping")
• (I think) then tabbing to another child control would fix itself and actually read the child control's name and help text
  By not adding the auto props to the panels, we completely skip the second bullet point above. So it works much better. Another good example to look at here is Defaults > Command line.

As for something like Interaction > Word Delimiters, I still think this is fine, but I want to verify it with a few screen reader users. Working on sending them a build. The behavior now is...

• on expander: "word delimiters, grouping, , collapsed"
• invoke expander: "expanded"
• tab to text box: "word delimiters, edit , "
  The key words here are "grouping" and "edit". We need the auto props on the expander so that the user knows what to expand/collapse (basically an opt-in for editing the setting). But we also need the same auto props on the text box so that the user knows what setting/field is specifically being edited (and what value is present). Removing one or the other is definitely problematic. And I would even go as far to argue that the name should be the same because you want the association between the expander, the setting, and the text field itself. Even as a sighted user, I'd say the same general flow exists right? It's like this: I want to edit setting X --> read expander --> click it to opt-in to modifying the value. I'd argue the same association exists?

Regardless, again, sending a build to a few people for feedback.

[carlos-zamora]

Yup! Verified with JAWS user Alex Benoit. It's 100% ok to have multiple controls have the same name as long as they are exposed as different controls. Since it's being read as "expanded/collapsed" (expander) vs "edit" (text box), he understands the UI tree.

[lhecker]

SGTM given your most recent comments.

[DHowett]

Eh, til::some has a hard upper bound (like std::array) but is dynamic below that bound (like std::vector). There's no way to make it heap allocate. I prefer it as a separate type! IMO!

[lhecker]

A potential counter point is that til::some vs. til::small_vector is a bit like FAIL_FAST_IF vs. assert. If you do make a mistake handling til::some it might crash you executable. Uncaught exceptions in coroutines for instance result in a std::terminate() IIRC.

[zadjii-msft]

sure, you tested it with someone who actually uses this everyday? that's better than any review I could give.

[DHowett]

Now, just conflicts to fix! I bet it's "removing the redundant tooltips"

[ghost]

Hello @carlos-zamora!

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.

[ghost]

🎉Windows Terminal v1.15.3465.0 and v1.15.3466.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download

[ghost]

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

Handy links:

• Release Notes
• Store Download