electron-builder icon indicating copy to clipboard operation
electron-builder copied to clipboard

use macOS-generated document icons by default

Open jeremyspiegel opened this issue 1 year ago • 2 comments

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] 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.

jeremyspiegel avatar Jun 28 '22 23:06 jeremyspiegel

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?

mmaietta avatar Jul 04 '22 04:07 mmaietta

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.

jeremyspiegel avatar Jul 06 '22 18:07 jeremyspiegel