Skip to content

Exit dialog window - #9652

Open
TheProjectDark wants to merge 1 commit into
apache:masterfrom
TheProjectDark:master
Open

TheProjectDark wants to merge 1 commit into
apache:masterfrom
TheProjectDark:master

Conversation

@TheProjectDark

@TheProjectDark TheProjectDark commented Oct 2, 2026 •

Copy link
Copy Markdown

Description

I was reviewing code in NetBeans and often miss-clicked cmd+q instead of pressing cmd+a, and it was so annoying to wait until the IDE re-launches so I just implemented exit dialog window like in Firefox or JetBrains.

Implementation

The exit flow is centralized in ExitDialog.showDialog(), which is already called during shutdown. It checks for open unsaved files first: if there are any, NetBeans shows its existing save/discard dialog. If there are none, it checks the new preference and, when enabled, shows Exit, Save All and Exit, and Cancel. Choosing Save All calls LifecycleManager.saveAll() before allowing shutdown; Cancel keeps the IDE open.
The checkbox is in Tools > Options > General. The panel loads its value when the options page opens, detects changes so Apply is enabled when appropriate, and saves the preference when settings are applied. The preference is stored with NetBeans’ user settings and defaults to enabled. Cmd+Q and Ctrl+F4 reach the same shutdown flow through their respective platform handlers/keymap bindings.

Screenshots

Screenshot 2026-10-01 at 14 08 34 Screenshot 2026-10-01 at 14 08 14

Assisted by: GitHub Copilot to navigate through project's structure


^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

LLMs, Commit messages and PR description:

  • Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.
  • LLM assisted commits should be attributed with an Assisted-by: MODEL_NAME MODEL_VERSION line appended to the commit message.
    • Please mention coding assistance in the PR description too (eg. by adding the same Assisted-by line from above)
    • Please describe the changes in your own words - we'd like to know you understand the changes being made!

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

@neilcsmith-net neilcsmith-net left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for looking at this. I hit this issue sometimes with a dual monitor setup when the close icon is right next to something I'm trying to click on the other screen! 😄

Couple of changes that are not related need looking at, and I'm not sure why we need the Save All option?

Comment thread ide/defaults/src/org/netbeans/modules/defaults/mf-layer-eclipse-keybinding.xml Outdated
Comment thread ide/defaults/src/org/netbeans/modules/defaults/mf-layer.xml Outdated
Comment thread platform/o.n.core/src/org/netbeans/core/ExitDialog.java Outdated
@neilcsmith-net neilcsmith-net added the UI User Interface label Oct 2, 2026

# Exit confirmation shown when there are no open files with unsaved changes
TTL_ExitConfirmation=Exit NetBeans
MSG_ExitConfirmation=Do you want to exit NetBeans?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's leave the word "NetBeans" out of these and keep the messages generic. Less surprising for platform applications, and we can always brand them later in the nb cluster if need be.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed to IDE

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I had considered something even more generic than "IDE" (eg. ".. application" or just "Do you want to exit?") given that not all platform applications are IDEs. Although, as mine is, I won't complain too much if other people are happy with that? 😄

@TheProjectDark

Copy link
Copy Markdown
Author
Screenshot 2026-10-02 at 13 44 44

So far I implemented the requested changes

@mbien mbien added the ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) label Oct 2, 2026
@neilcsmith-net

Copy link
Copy Markdown
Member

Thanks. Principle looks good to me, but not tested yet. Triggered CI and a dev build. Will wait for feedback from others.

Everything will need squashing into a single commit and force pushing before it could be merged. But hold fire on that in case there's other feedback first.

@ebarboni

ebarboni commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

LGTM to me.

Author in commit expect 2 parts.

@mbien

mbien commented Oct 5, 2026

Copy link
Copy Markdown
Member

tested a scenario which would open both exit popups in sequence.

run project with

    static void main() throws InterruptedException {
        System.out.println("sleeping");
        Thread.sleep(Duration.ofMinutes(2));
    }

try to exit IDE.

new popup:
image

old popup:
image

this worked, but the visually confusing aspect is that it looks like the new dialog has the cancel button selected, but focus is on exit. Probably would have to play with the NotifyDescriptor config a bit to make both visually consistent. (the old dialog will select a list item on enter - which isn't great either tbh)

Comment thread platform/o.n.core/src/org/netbeans/core/ExitDialog.java Outdated
# Exit confirmation shown when there are no open files with unsaved changes
TTL_ExitConfirmation=Exit IDE
MSG_ExitConfirmation=Do you want to exit the IDE?
CTL_ExitConfirmationExit=Exit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@neilcsmith-net should we change this to "Exit IDE" like on the other dialog? #9652 (comment)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be honest, I'd probably have "Exit" and "Do you want to exit?" instead. That's with the perspective of having the platform messages here as generic as possible, and consistent, so they might not need branding. I know the running tasks one from core.execution already uses IDE though. So I don't mind either way really - my platform app is an IDE, so that's not a concern for me.

@TheProjectDark
TheProjectDark force-pushed the master branch 2 times, most recently from a50f18a to 5cd3c2b Compare October 5, 2026 19:57
@neilcsmith-net

neilcsmith-net commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

@mbien how did you test that scenario with the new vs old popup? Using ant tryme with netbeans.full.hack?

This scenario might be a little more complicated to get right. We have the various @OnStop registered Callable that might show a dialog, like the one you bring up. These are called after the ExitDialog. Ideally we want anything that's trapping exit to only show if no other dialog has intervened first. And I'm not sure we have a good way to do that at the moment.

The dialog is also showing up strangely for me at times, not like the screenshot, but with the buttons vertically stacked to the right of the label. That might be issues we sometimes see with chaining multiple dialogs in the EDT.

Sorry, @TheProjectDark now we've got to testing this we might be facing more edge cases than envisaged. I would still like us to get this feature in if we can work out how.

@neilcsmith-net

Copy link
Copy Markdown
Member

Yes, there's definitely something weird with the look of the last push on my system -

Screenshot from 2026-10-06 14-13-32

I would suggest reverting to your previous iteration, but with exit as the default value. eg.

            Object exit = bundle.getString("CTL_ExitConfirmationExit");
            Object cancel = bundle.getString("CTL_ExitConfirmationCancel");
            NotifyDescriptor descriptor = new NotifyDescriptor(
                    bundle.getString("MSG_ExitConfirmation"),
                    bundle.getString("TTL_ExitConfirmation"),
                    NotifyDescriptor.YES_NO_OPTION,
                    NotifyDescriptor.QUESTION_MESSAGE,
                    new Object[] { exit, cancel },
                    exit);
            Object choice = DialogDisplayer.getDefault().notify(descriptor);
            return exit.equals(choice);

Having Cancel as default works strangely - it's highlighted but pressing Enter still exits (as it should). This feels like a disconnect between keyboard usage of Enter and Escape, and what you can see on screen.

I think it might be better to default to disabled for the coming release as well. We can consider enabling by default at a later date.

Had a look through other registered @OnStop handlers and not sure there's any but the running tasks one that will also show. Having both in that scenario is probably OK.

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

Labels

ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) UI User Interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants