Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Match SearchBox AutoProp.Name to placeholder text #14519

Merged
2 commits merged into from
Dec 13, 2022
Merged

Match SearchBox AutoProp.Name to placeholder text #14519

2 commits merged into from
Dec 13, 2022

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Dec 9, 2022 •
edited
Loading

Voice Access allows for functionality like "click <name>" to automatically move the cursor and click on a control. The Search Box had an AutoProp.Name that didn't match the placeholder text, leading to confusion because Voice Access wouldn't be able to find a control named "Find...".

To fix this, we simply aligned the placeholder text and the AutoProp.Name to be "Find". The elipses were removed because no dialog is opened.

This fix was accomplished by modifying the resources file to explicitly match the AutoProp.Name to the placeholder text.

Closes #14398

@carlos-zamora
Copy link
Member Author

@DHowett Localization is still a bit of a mystery to me, so let me know if you want me to remove one of the two implementations I have here. Figured I'd cover my bases haha.

@carlos-zamora
Copy link
Member Author

This PR does NOT close #14398. The issue says that the same issue is seen with combo boxes in our app, and I'm asking for clarification there.

DHowett
DHowett requested changes Dec 10, 2022
View reviewed changes
src/cascadia/TerminalControl/Resources/en-US/Resources.resw
<!--
Microsoft ResX Schema

<!--
Copy link
Member

Choose a reason for hiding this comment

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

how did we accept another change that broke this file???

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought any time we use VS's table thing, it always rewrites the top part.

Copy link
Member

Choose a reason for hiding this comment

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

It can't always rewrite it, it just always adds back these spaces and spelling errors if they're missing. :)

src/cascadia/TerminalControl/SearchBoxControl.cpp Outdated
@@ -4,6 +4,7 @@
#include "pch.h"
#include "SearchBoxControl.h"
#include "SearchBoxControl.g.cpp"
#include <LibraryResources.h>
Copy link
Member

Choose a reason for hiding this comment

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

You should only need the resw one for this, because it will automatically populate the attached property when it loads the resource tree. If that isn't working, ... then that sucks. Let me know.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it works. This was more because I have trust issues; like, this would work in the event the localization team doesn't read "This should match 'SearchBox_TextBox.PlaceholderText'" haha

@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 10, 2022
@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Dec 13, 2022
@ghost
Copy link

ghost commented Dec 13, 2022

Hello @DHowett!

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 ghost added Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Dec 13, 2022
@ghost ghost merged commit f9a36f1 into main Dec 13, 2022
@ghost ghost deleted the dev/cazamor/a11y/find-auto-prop branch December 13, 2022 21:40
@carlos-zamora carlos-zamora mentioned this pull request Dec 15, 2022
Merged
ghost pushed a commit that referenced this pull request Dec 15, 2022
@carlos-zamora
Match 'add appearance' button text to AutoProp.Name (#14564)
dbc27ab 
Similar to #14519.

Voice Access allows for functionality like "click <name>" to automatically move the cursor and click on a control. The Search Box had an AutoProp.Name that didn't match the button text, leading to confusion because Voice Access wouldn't be able to find a control named "Create".

To fix this, we simply aligned the button text and the AutoProp.Name.

Closes #13808
@ghost
Copy link

ghost commented Jan 24, 2023

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

Handy links:

@ghost ghost mentioned this pull request Jan 24, 2023
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DHowett DHowett DHowett approved these changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

Assignees

@DHowett DHowett

Labels
Area-Accessibility Issues related to accessibility AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. zBugBash-Consider
Projects
None yet
Milestone
No milestone
3 participants
@carlos-zamora @DHowett @zadjii-msft