-
Notifications
You must be signed in to change notification settings - Fork 56
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
Avoid closing IDE accidentally #390
Avoid closing IDE accidentally #390
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 this. I like the idea but would suggest the following changes:
- use onDidChangeActiveTextEditor instread of the tab group change event
- register the event in Tabpanel instead of extension.ts and make sure we call dispose on the result of registration when dispose is triggered on the panel
- Use panel.active field to tell whether the IDE panel is focused (https://code.visualstudio.com/api/references/vscode-api#WebviewPanel)
- Rename
isTabActive
toisTabPanelFocused
since word "active" is ambiguous in this context - Rename "doubleClickClose" to "closeWithConfirmation"
@kmagiera Thanks for the review. |
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.
This is much better. I'm ok with using onDidChangeViewState
if it works and gets triggered at the right moment. Only added one more comment inline and this should be good to go.
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.
This is great. Thanks for working on the requested changes!
Problem
Currently, there is a chance of accidentally closing the IDE panel and it can be an unpleasant experience
Solution
Simple confirmation popup to avoid headaches. The following popup only appears if users click
cmd+w/ctr+w
https://github.com/software-mansion/react-native-ide/assets/77052270/70c45f35-9d89-4b99-ad97-9f69e0cbc96e
#387