KittyCAD / modeling-app

The KittyCAD modeling app.
https://kittycad.io/modeling-app/download
MIT License
429 stars 37 forks source link

Fix: linux icon #4554

Closed TomPridham closed 2 days ago

TomPridham commented 6 days ago

~This updates the electron-builder.yml file to output .deb files as well as AppImage files. Also updates the build-apps.yml action to copy the new .deb files and send them to the publishing script~ Fixes the linux icon so it shows up in the dock and when searching for it Screenshot from 2024-11-23 13-43-06 Screenshot from 2024-11-23 13-43-13 I can't figure out how to get the icon to show up when installing the .deb file or in the file tree, however Screenshot from 2024-11-23 13-36-16 Screenshot from 2024-11-23 13-44-15

qa-wolf[bot] commented 6 days ago

QA Wolf here! As you write new code it's important that your test coverage is keeping up. Click here to request test coverage for this PR!

vercel[bot] commented 6 days ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Nov 25, 2024 8:06pm
pierremtb commented 4 days ago

Hey @TomPridham, thanks for the PR! Sorry realizing now that I didn't follow up on the issue after that comment where I was implying we would want to go for .deb packages as well as .AppImage. After talking to the team we agreed on not moving forward with .deb support at least for now, as it adds to the list of things to test and support, which we're trying to keep small for now.

Was the 256x256.png icon addition needed on your side to get the icon to show up when running the AppImage file? I believe it's the only change not .deb related here.

TomPridham commented 3 days ago

Yeah, adding that 256x256.png icon was the only thing necessary to get the icon to show up. Given that it doesn't seem like the .deb files will even work out of the box right now until an Electron update, that makes sense. I reverted those changes

pierremtb commented 3 days ago

@TomPridham Thanks! I'm not 100% sure the added 256x256 file is what's getting the icon to show up on linux though, just tried on a fresh 24.10 install, building the latest from main this morning and the icon is already part of the image, see

pierremtb@xps13:~/Documents/modeling-app$ ls /tmp/.mount_Zoo\ MohVAY7S/
AppRun                    chrome-sandbox            libffmpeg.so              LICENSE.electron.txt      resources.pak             vk_swiftshader_icd.json   
chrome_100_percent.pak    .DirIcon                  libGLESv2.so              LICENSES.chromium.html    snapshot_blob.bin         zoo-modeling-app          
chrome_200_percent.pak    icudtl.dat                libvk_swiftshader.so      locales/                  usr/                      zoo-modeling-app.desktop  
chrome_crashpad_handler   libEGL.so                 libvulkan.so.1            resources/                v8_context_snapshot.bin   zoo-modeling-app.png 

but it wasn't showing up until I installed appimaged, the service that's supposed to help integrating appimages in the desktop environment. Likely just linking that desktop file in the image.

This is also what this doc is implying https://www.electron.build/icons.html#linux, that icon.png is enough and is converted by the tool. And frankly I'd like to keep to one source of truth for the icon if that works!

TomPridham commented 2 days ago

Yeah, it just works in Ubuntu 24 for me, even without installing that extra service. I had only checked on 22 before. Installing that service was required for the icon to show up when I tested it just now on 22 on a USB. I'll go ahead and close this since it isn't necessary anymore

pierremtb commented 2 days ago

Awesome, thanks for testing all of that!