-
Notifications
You must be signed in to change notification settings - Fork 6.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
[Peek][PreviewPane] Fix missing Copy menu-item #33845
Conversation
@drawbyperpetual Not sure how it looks. But for the layout a separator line between the copy command and the setting toggle entries would be good, I think. |
@htcfreek : I've added a separator between the commands and also updated the PR description with screenshots. Let me know what you think! |
Looks good. |
Btw, merging this should be synced with merging #33742. |
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.
Thanks for the feature.
LGTM. Only quirk I've found is that if I use Peek on either Markdown or Monaco files, make the context menu show and then move the Peek window between monitors, it will make the context menu start appearing on random spots. Like sometimes it even shows on a different monitor than what the Peek window is in.
…ith WebView2 menu
@jaimecbernardo : I've fixed that context menu location issue for Peek. The way I did it was to not rely on the WebView2 context menu at all but instead draw a WPF context menu instead. The only downside is I couldn't find a way to replicate the pretty Copy icon from the WebView2 context menu. But I think that's a small issue as Monaco doesn't have it either. BTW, the PreviewPane context menu doesn't seem to have to that context menu location issue for whatever reason. So I've kept the WebView2 context menu for the PreviewPane + Markdown previewer instead of drawing a WPF context menu. |
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.
LGTM! Thank you!
Summary of the Pull Request
Fixes two bugs:
PR Checklist
Detailed Description of the Pull Request / Additional comments
The issues are:
Validation Steps Performed