electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.73k stars 1.74k forks source link

use macOS-generated document icons by default #6966

Open jeremyspiegel opened 2 years ago

jeremyspiegel commented 2 years ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch app-builder-lib@23.1.0 for the project I'm working on.

If I don't specify a custom icon for my file association, then app-builder-lib will default to use the icon file for the main app bundle, which results in the icon for the file being the same as for the application. Instead, I think it should allow macOS to generate a document icon as a piece of paper with its top-right corner folded down, including the application icon and file extension (https://developer.apple.com/design/human-interface-guidelines/foundations/icons#document-icons/).

Here is the diff that solved my problem:

diff --git a/node_modules/electron-builder/node_modules/app-builder-lib/out/electron/electronMac.js b/node_modules/electron-builder/node_modules/app-builder-lib/out/electron/electronMac.js
index cfe3729..b2f745f 100644
--- a/node_modules/electron-builder/node_modules/app-builder-lib/out/electron/electronMac.js
+++ b/node_modules/electron-builder/node_modules/app-builder-lib/out/electron/electronMac.js
@@ -176,7 +176,7 @@ async function createMacApp(packager, appOutDir, asarIntegrity, isMas) {
                 CFBundleTypeName: fileAssociation.name || extensions[0],
                 CFBundleTypeRole: fileAssociation.role || "Editor",
                 LSHandlerRank: fileAssociation.rank || "Default",
-                CFBundleTypeIconFile: iconFile,
+                ...(customIcon && {CFBundleTypeIconFile: iconFile}),
             };
             if (fileAssociation.isPackage) {
                 result.LSTypeIsPackage = true;

This issue body was partially generated by patch-package.

mmaietta commented 2 years ago
let iconFile = appPlist.CFBundleIconFile

iconFile is being set by the appPlist. Do you know where that Info.plist is coming from? I feel like we should be setting it there instead of only checking if customIcon is present. Your patch would be like a breaking change, right?

jeremyspiegel commented 2 years ago

I'm not 100% but I think that Info.plist comes from the downloaded Electron bundle, and it's used as a template and modified by electron-builder, so I don't think setting it there is an option.

Your patch would be like a breaking change, right?

My patch changes the default behavior (when customIcon isn't set) from using the application icon to allowing macOS to generate a document icon. This is a "breaking change" in the sense that it does change behavior, but it seems more correct to me than the previous behavior. We'd need to also change the docs for fileAssociations.icon to match, or instead change the configuration to allow a way to opt in to this new behavior.

github-actions[bot] commented 20 hours ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.