-
Notifications
You must be signed in to change notification settings - Fork 53
Add menu item to make the Studio to float on top of other windows #511
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
fluiddot
left a comment
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 🎊 ! Tested on macOS and Windows.
I added a comment regarding where to place the new item but shouldn't block the PR.
katinthehatsite
left a comment
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.
The changes look good and the functionality works as expected. It would be nice to move this to the Window section as suggested by another comment
|
@fluiddot , @katinthehatsite , Thanks for reviewing it. I've updated the code to fix a bug in Windows, I would appreciate a second review. I decided to put it under View to follow other apps like QuickTime. Let me know what you think. The change is trivial. While testing on Windows, I've noticed the toggle was not working. Once the user clicked on the menu item, it was not "checked" and the window was floating constantly even after clicking the menu item for second or third time. Fixed here: 99908d1 |
@sejas Ah, interesting. I didn't encounter this when I tested it on Windows. What Windows version have you tested it? |
|
@fluiddot , I'm using Windows 11 Home. I've observed that the menu in Windows is being "rendered" inside the App, and that's probably why the auto-toggle is not working out of the box. I encountered a couple of related issues:
They solve it by calling |
fluiddot
left a comment
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 🎊 ! I've re-tested it in both macOS and Windows and worked nice.
Proposed Changes
Testing Instructions
npm startQJa8NX.mp4
Pre-merge Checklist