flathub / org.claws_mail.Claws-Mail

https://flathub.org/apps/details/org.claws_mail.Claws-Mail
4 stars 1 forks source link

Icon doesn't show up #2

Closed bilelmoussaoui closed 4 years ago

bilelmoussaoui commented 4 years ago

https://flathub.org/apps/details/org.claws_mail.Claws-Mail

Please also remove the icon cache file. It's generated on the user's system and should not be shipped here.

Also, please install the appdata file only once in metainfo https://github.com/flathub/org.claws_mail.Claws-Mail/blob/master/org.claws_mail.Claws-Mail.json#L243-L244 no need to install it on appdata folder too.

The icons seems to be installed using a different icon name, it should use the app-id and you will probably need to patch the desktop file too. You can use the rename-icon & copy-icon properties of the flatpak manifest.

cobratbq commented 4 years ago

Please also remove the icon cache file. It's generated on the user's system and should not be shipped here.

Done.

Also, please install the appdata file only once in metainfo https://github.com/flathub/org.claws_mail.Claws-Mail/blob/master/org.claws_mail.Claws-Mail.json#L243-L244 no need to install it on appdata folder too.

Done. I wasn't sure as different packages have different approaches (apparently).

The icons seems to be installed using a different icon name, it should use the app-id and you will probably need to patch the desktop file too. You can use the rename-icon & copy-icon properties of the flatpak manifest.

Desktop icon (finally) fixed. I haven't used rename-icon or copy-icon. I think it would help if you do a quick check and let me know if things can be improved. Right now I've copied the desktop file from the claws-mail package after build, so a little bit of redundancy.

bilelmoussaoui commented 4 years ago

You could use desktp-file-edit to edit the desktop file on post-install. See https://github.com/flathub/com.github.optyfr.JRomManager/blob/0b1681e54b2aa2ecf5d6bc3d5724f6cae4d2e67b/com.github.optyfr.JRomManager.yaml#L30

Your current patch is not working as it's applied to the desktop file from the debian package but you're installing the destkop file shipped in this repository.

What you will need to do:

cobratbq commented 4 years ago

Your current patch is not working as it's applied to the desktop file from the debian package but you're installing the destkop file shipped in this repository.

Is there a build log where I can see results? For me, locally, the patch is applied to the desktop-file I just committed in the repo. (Building the package fails now that I removed it.)

bilelmoussaoui commented 4 years ago

In that case, just remove the patch and apply it directly the shipped desktop file.

bilelmoussaoui commented 4 years ago

Also, you can use merge requests to trigger bot builds & ensure things are working before publishing directly.

bilelmoussaoui commented 4 years ago

The icon shows up correctly now. Thansk! It would be nice if we could update those pre 2000 screenshots if possible

cobratbq commented 4 years ago

It would be nice if we could update those pre 2000 screenshots if possible

@bilelmoussaoui That is still an item I intend to fix. Also the resolutions are wrong. How strict are the expectations of using a 16:9 ratio resolution? (Because a compose window wouldn't look very nice with such a ratio.)

bilelmoussaoui commented 4 years ago

I have never used the 16:9 ratio myself on my apps, just make sure the screenshots are up to date so the user gets what he sees on the screenshots.