-
Notifications
You must be signed in to change notification settings - Fork 315
Support tooltip rendering in the prediction list view #3667
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
Conversation
Love the UI! Is there a way to disable the tooltip? |
Same as disabling tooltip in menu completion today: |
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.
Can't give a proper code review but the design and UX look great to me!
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.
Looks good to me!
{ | ||
const int LengthOfLeadingPart = 6; | ||
const string IndicatorSymbol = ">>"; | ||
const string MsgForViewAll = "(<F4> to view all)"; |
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.
Just curious if we do this elsewhere. I suspect that key binding is customizable by the user, but this won't update. Is that the case?
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.
Right, the key could already be bound by a user and this message currently won't update in that case.
We don't do this elsewhere -- first time advertising a key-binding in the rendering. Will collect the feedback and see how to best deal with it.
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.
Opened #3668 to track this.
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.
Nice test.
PR Summary
Support tooltip rendering in the prediction list view. Fix #3638.
By default, the list view can use up to 4 lines for tooltip rendering:
For tooltip that spans more than 4 lines, we allow the user to see the full tooltip in the alternate screen buffer with the keybinding F4
When terminal size shrinks, the maximum lines can be used for tooltip also decreases -- it uses up to 2 lines for tooltip when the max list height is reduced to 5.
When the terminal size is even smaller and the max list view height becomes 3, it uses up to 1 line for tooltip.
You can change the tooltip color by
Set-PsreadLineOption -Colors @{ ListPredictionTooltip = "green" }
, for example.PR Checklist
Microsoft Reviewers: Open in CodeFlow