Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Add prevent option to ::onWillDestroyPaneItem#22943

Merged
sadick254 merged 1 commit into
atom:masterfrom
ericcornelissen:prevent-destroy-pane-item
Sep 7, 2021
Merged

Add prevent option to ::onWillDestroyPaneItem#22943
sadick254 merged 1 commit into
atom:masterfrom
ericcornelissen:prevent-destroy-pane-item

Conversation

@ericcornelissen

Copy link
Copy Markdown
Contributor

Issue or RFC Endorsed by Atom's Maintainers

#13812

Description of the Change

This PR implements the feature requested in issue #12376. It adds a method to the parameter of the ::onWillDestroyPaneItem callback named prevent which allows packages to prevent a tab from closing by adding an event listener via ::onWillDestroyPaneItem.

Alternate Designs

An alternative for the requested feature overall isn't really present, as the previous implementation would close the tab no matter what or request the user to safe in case of unsaved changes. Both of these are unwanted behaviour as described in #12376.

As for alternative ways to implement the functionality, I considered prevent to be a (boolean) value instead of a function. However, this seemed not such a good choice mainly because I find it more intuitive to use as a function, but also because JavaScript can't enforce the value to be a boolean.

Possible Drawbacks

When used incorrectly this functionality could make closing tabs impossible for users. However, simply disabling the package that uses this functionality incorrectly will solve the problem.

Verification Process

I tested the functionality by using it in the pinned-tabs-for-atom package when I created #13812. I have not yet been able to retest it yet.

Release Notes

Not applicable

@sadick254

Copy link
Copy Markdown
Contributor

@ericcornelissen Kidly take a look at the failing specs on the CI.

@ericcornelissen

Copy link
Copy Markdown
Contributor Author

Should be all good now @sadick254, I had forgotten to remove a line when I moved the changes from #13812 over to this new branch 😅

@sadick254 sadick254 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@sadick254 sadick254 merged commit b3930b0 into atom:master Sep 7, 2021
@ericcornelissen ericcornelissen deleted the prevent-destroy-pane-item branch September 7, 2021 08:08
@asturur

asturur commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

I moved to latest nightly (19) and atom is all broken for me now.
image

is this PR related?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants