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: linux icon #4554

Closed
wants to merge 7 commits into from
Closed

Fix: linux icon #4554

wants to merge 7 commits into from

Conversation

TomPridham
Copy link
Contributor

@TomPridham TomPridham commented Nov 23, 2024

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

Copy link

qa-wolf bot commented Nov 23, 2024

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!

Copy link

vercel bot commented Nov 23, 2024

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

@@ -325,6 +326,8 @@ jobs:
--arg mac_x64_url "$RELEASE_DIR/${{ env.URL_CODED_NAME }}-${VERSION_NO_V}-x64-mac.dmg" \
--arg windows_arm64_url "$RELEASE_DIR/${{ env.URL_CODED_NAME }}-${VERSION_NO_V}-arm64-win.exe" \
--arg windows_x64_url "$RELEASE_DIR/${{ env.URL_CODED_NAME }}-${VERSION_NO_V}-x64-win.exe" \
--arg deb_arm64_url "$RELEASE_DIR/${{ env.URL_CODED_NAME }}-${VERSION_NO_V}-arm64-linux.deb" \
--arg deb_amd64_url "$RELEASE_DIR/${{ env.URL_CODED_NAME }}-${VERSION_NO_V}-amd64-linux.deb" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why, but instead of an x86 file, I get an amd64 file

@TomPridham TomPridham changed the title Output .deb files as well as AppImage files for Linux Output .deb files as well as AppImage files for Linux, fix linux icon Nov 23, 2024
@pierremtb
Copy link
Collaborator

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 TomPridham marked this pull request as ready for review November 25, 2024 19:55
@TomPridham
Copy link
Contributor Author

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

@TomPridham TomPridham changed the title Output .deb files as well as AppImage files for Linux, fix linux icon Fix: linux icon Nov 25, 2024
@pierremtb
Copy link
Collaborator

@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!

@pierremtb pierremtb self-requested a review November 26, 2024 16:25
@TomPridham
Copy link
Contributor Author

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

@TomPridham TomPridham closed this Nov 26, 2024
@TomPridham TomPridham deleted the fix/linux-build branch November 26, 2024 19:20
@pierremtb
Copy link
Collaborator

Awesome, thanks for testing all of that!

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