Skip to content
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

Fix: Add custom About dialog for non-macOS platforms (#421) #1175

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SidheshwarSarangal
Copy link

@SidheshwarSarangal SidheshwarSarangal commented Jan 23, 2025

Summary:
This pull request addresses Issue #421, which involves fixing the behavior of the "About" menu item in the application. Currently, the "About" dialog is not functioning as expected on non-macOS platforms. The fix ensures that the "About" dialog appears as a modal window for all platforms except macOS, where the native "About" dialog behavior is retained.

Description for the changelog:
Fixed the "About" menu behavior: non-macOS platforms now open a custom modal window, while macOS retains the native About dialog.

Other info:
This PR closes #421 and ensures the "About" menu item behaves appropriately across different platforms.

For macOS, the menu uses the default system behavior.
For non-macOS platforms, the "About" item opens a custom modal window displaying about.html.
This change ensures a more consistent and expected behavior for the "About" dialog across all platforms.

@jgadsden jgadsden added the enhancement New feature or request label Jan 24, 2025
@jgadsden jgadsden self-requested a review January 24, 2025 05:30
Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

hello @SidheshwarSarangal , thanks for this update, it is good to have
could you run the unit tests with npm test and make sure they pass?

@jgadsden jgadsden marked this pull request as draft January 24, 2025 07:24
@SidheshwarSarangal SidheshwarSarangal marked this pull request as ready for review January 24, 2025 09:11
parent: mainWindow, // Use mainWindow as the parent if applicable
resizable: false,
});
aboutWin.loadFile('about.html'); // Load the 'about.html' file in the modal
Copy link
Collaborator

Choose a reason for hiding this comment

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

does the about.html need to be created?

Copy link
Author

Choose a reason for hiding this comment

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

I have checked. Currently, there is no about.html file in the repository. Should I create one for this task? If so, could you confirm the expected content or structure?

Copy link
Author

Choose a reason for hiding this comment

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

I have added the about content. Please tell me if there is anything else.

@jgadsden jgadsden marked this pull request as draft January 27, 2025 07:04
@SidheshwarSarangal SidheshwarSarangal marked this pull request as ready for review January 27, 2025 10:17
@SidheshwarSarangal
Copy link
Author

I have added the about content. Please tell me if there is anything else.

Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

I think there may be a better way to implement this, for example electron-about-window, or other ways of achieving this cross platform.
This is a commonly needed feature across most electron applications so there is probably going to be an easy path somewhere

@jgadsden jgadsden marked this pull request as draft January 28, 2025 07:23
@SidheshwarSarangal SidheshwarSarangal marked this pull request as ready for review January 28, 2025 16:08
@SidheshwarSarangal
Copy link
Author

Sir, I have used about-window for non mac os

Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

hello @SidheshwarSarangal , could you look at your changes again? they do not seem to use package electron-about-window, see electron-about-window main.js for an example

@jgadsden jgadsden marked this pull request as draft February 3, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

desktop about-box is MacOS only
2 participants