Skip to content

Conversation

@brtnfld
Copy link

@brtnfld brtnfld commented Jan 16, 2026

Isolated fix from #454


Important

Fixes macOS menu handling in HDFView.java and centralizes macOS detection in ViewProperties.java.

  • Behavior:
    • Adds setupMacOSMenuHandlers() in HDFView.java to handle macOS-specific "About" and "Preferences" menu items.
    • Refactors user options dialog creation into openUserOptionsDialog() in HDFView.java.
  • OS Detection:
    • Adds isMacOS() method in ViewProperties.java for centralized macOS detection.
    • Replaces inline macOS checks with ViewProperties.isMacOS() in DefaultScalarDSTableView.java and HDFView.java.
  • Code Owners:
    • Updates .github/CODEOWNERS to replace @byrnHDF with @mattjala.

This description was created by Ellipsis for e6b15a5. You can customize this summary. It will automatically update as commits are pushed.

@brtnfld brtnfld marked this pull request as draft January 16, 2026 18:43
@brtnfld brtnfld marked this pull request as ready for review January 16, 2026 20:33
@brtnfld brtnfld marked this pull request as draft January 16, 2026 20:35
userOptionDialog.getShell().isDisposed());

if (needsCreation) {
log.debug("Creating User Options dialog (first open or after disposal) - this takes ~2 seconds...");
Copy link
Collaborator

Choose a reason for hiding this comment

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

It really doesn't seem appropriate to mention anything about timing even in logging, especially because opening this dialog is near-instant for me on Linux.

userOptionDialog.create();
}
else {
log.debug("Reusing cached User Options dialog (instant open)");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here as well

Copy link
Author

Choose a reason for hiding this comment

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

I think I'm going to abandon trying to work around the delay on the mac, nothing seems to work. It just pushes the delay somewhere else.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Some initial profiling should be done to see where time is being spent. There's a lot of logic that can probably be avoided, especially if we separate options into categories better.

@brtnfld brtnfld requested a review from jhendersonHDF January 16, 2026 21:51
@brtnfld brtnfld marked this pull request as ready for review January 16, 2026 21:51
Removed OS-specific guard - No more ViewProperties.isMacOS() check
Removed unnecessary asyncExec
Removed redundant checks - No need for shell.isDisposed() checks that aren't used elsewhere
Cleaned up variables - Removed unused display variable
Platform-neutral naming - Method and log messages now use "system menu" terminology
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants