-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
Feat preview color scheme on mouse hover #18518
base: main
Are you sure you want to change the base?
Feat preview color scheme on mouse hover #18518
Conversation
I previously wrongly triggered pointer routed events when the mouse hovered the ListView object instead of its individual items.
@microsoft-github-policy-service agree |
I added a 10ms delay with a DispatcherTimer to debounce pointer exit events because the preview updates were clunky when quickly moving the pointer between ListViewItems.
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 and feels great! Just the one comment before approving. Thanks for doing 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.
(whoops, clicked the wrong button...)
Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com>
Updated implementation to call PreviewAction on all command palette items instead of only submenu items, following PR feedback.
@DHowett can you please take a look when you have the chance? Thanks! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@eleadufresne so sorry for the delay! I've been out sick this week, but I'm putting it at the top of my list |
ah, you may need to run the code formatter :) i am dismayed that we are no longer allowed to run these things without team approval - it slows down the whole PR process. |
@DHowett Thanks for taking the time to look at it—especially while you’re not feeling great—and apologies for missing this in my last commit. I ran the formatter, so fingers crossed it passes all the checks this time! By the way, can I run the pipeline myself now, or should it always be handled by someone from the team? |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
unfortunately, it still needs a team member to approve each iteration |
Ah gotchu, thanks for the info! |
@DHowett, looks like all checks have passed 😄 Thanks again for your help! Let me know if you need anything else before merging. |
Summary of the Pull Request
Allow users to preview color schemes by hovering over them with the mouse pointer in the Command Palette.
References and Relevant Issues
UP
andDOWN
arrows to trigger a preview of color schemes in the Command Palette.Detailed Description of the Pull Request / Additional comments
This works by attaching event handlers for
PointerEntered
andPointerExited
toListViewItem
containers. When the mouse pointer moves into the item's bounding area, thePreviewAction
handler is triggered to showcase the hovered color scheme. Conversely, when the mouse pointer leaves the item's area, thePreviewAction
is executed on the selected item (generally from theUP
andDOWN
arrows).Decisions I made, but I'm uncertain about:
I coded my solution so thatPreviewAction
is called on the submenu items, as this is where the color schemes are located. I felt invoking the function for each element was unnecessary, but this approach may make it less flexible for future modifications.ListViewItem
changes, but there might be a better approach.Important note:
ActionPreviewHandler
handles, such as the background opacity of the terminal.Validation Steps Performed
ESC
at any point to dismiss the command palette, and the scheme returns to the previous one.PR Checklist