-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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.
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
Add tooltips to the Suggestions Control #14939
Add tooltips to the Suggestions Control #14939
Conversation
…grie/f/3121-tooltips
…grie/f/3121-tooltips
This comment has been minimized.
This comment has been minimized.
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
…grie/f/3121-tooltips
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessibility is gonna be rough here. You should test it out, but I assume TeachingTips aren't read out by screen readers. What you could do though, is apply that text to the AutomationProperties.FullDescription
property. That way, the user can specifically request more info whenever they want it. That info shouldn't be read out automatically by the screen reader either. Only on request.
IsOpen="False" | ||
PreferredPlacement="Right" | ||
ShouldConstrainToRootBounds="False" | ||
Subtitle=""> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "Subtitle" necessary? Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with Title?
Also, looks like the WinUI docs don't say the default value (grr). So, mind checking each of these to make sure they're necessary?
gods I forgot they aren't. WHY ARENT THEY. Also, Narrator and the Command Palette are already weird. Like, navigating the cmdpal with narrator never moves the selected item. It moves focus, but not the selected item. So of course, for the tooltips, they too do not get updated with narrator open. Frick. |
Notes:
I've got a vibe that this is too wacky to land for 1.19. I'm gonna close this one for now and come back to it. Further reading:
|
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view or the 📜action log for details. Unrecognized words (2)exersize Previously acknowledged words that are now absentcountof cpprest homeglyphs keyevent ONLCR OSCBG OSCCT OSCFG OSCRCC OSCSCB OSCSCC OSCWT pplx pseudocode testdlls Tlgdata websockets xxyyzz :arrow_right:To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands... in a clone of the git@github.com:microsoft/terminal.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/5823618460/attempts/1' ✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉 If the flagged items are 🤯 false positivesIf items relate to a ...
|
AHAHAHA I finally got it to work right!
|
This adds a `"description"` property to actions. Notably, the shell completion protocol (#3121) will now also populate that. The suggestions UI can then use those descriptions to display an additional tooltip with that information. TeachingTip was kinda an abject disaster last time I tried this, so this _isn't_ a TeachingTip. It's literally a text block. xlinks: * #13000 * #15845 * #14939 - the last abandoned attempt at this
targets #14938
These are currently powered by the
"ToolTip"
element in the completion protocol. This also adds room for"description"
s in any action. This will be more relevant for Tasks, in #1595.By all accounts, this could have been in the parent commit. I broke it apart for the ease of reviewing.