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

[Feat] tauricon: Allow for additional custom generated icon files. #5121

Closed
phisch opened this issue Jul 9, 2022 · 14 comments
Closed

[Feat] tauricon: Allow for additional custom generated icon files. #5121

phisch opened this issue Jul 9, 2022 · 14 comments

Comments

@phisch
Copy link

phisch commented Jul 9, 2022

I have the use-case where I serve my front-end as a tauri app and also as a web-app with PWA support. Instead of maintaining two sets of icons, I would like to just generate one set that is used by everything. This is probably a pretty common use-case.

So it would be nice to have a way to define alternative icon formats, sizes and file-names, that are also generated by tauricon.

Currently, the files to be generated are defined here: https://github.com/tauri-apps/tauricon/blob/dev/src/helpers/tauricon.config.ts

Probably the easiest implementation would be to keep that same structure and provide it via the cli as a json file:

npx @tauri-apps/tauricon icons/logo.svg --target icons --extra icons/extra.json

And then the icons/extra.json could be something like this:

[
  {
    "folder": "../frontend/src/assets/icons",
    "prefix": "icon-",
    "infix": true,
    "suffix": ".png",
    "sizes": [72, 96, 128, 144, 152, 192, 384, 512]
  }
]

What are your thoughts about that?

@FabianLars FabianLars transferred this issue from tauri-apps/tauricon Sep 1, 2022
@FabianLars FabianLars changed the title Allow for additional custom generated icon files. [Feat] tauricon: Allow for additional custom generated icon files. Sep 1, 2022
@khuongduy354
Copy link
Contributor

Can I take this as a newbie? What i'm going to add in this file is:

  1. take path to extra.json as a cli argument
  2. import the json from path
  3. call another tauricon.make() with json as option parameter

@FabianLars
Copy link
Member

We always appreciate contributions! The only problem here is that the tauricon (js) package was deprecated and we won't accept any PRs for it anymore (the repo will be archived soon too).
As of tauri-cli v1.1 tauricon is now part of the main cli again - relevant file: https://github.com/tauri-apps/tauri/blob/dev/tooling/cli/src/icon.rs

@FabianLars
Copy link
Member

FabianLars commented Sep 17, 2022

Btw idk how other people see it but i'm not too happy about the prefix/suffix json bloat. What do you guys think about an optional {} in the filename which would be replaced with sizexsize?

  {
    "folder": "../frontend/src/assets/icons",
    "name": "icon-{}.png", // would be icon-72x72.png for example.
    "sizes": [72, 96, 128, 144, 152, 192, 384, 512]
  }

@khuongduy354
Copy link
Contributor

Btw idk how other people see it but i'm not too happy about the prefix/suffix json bloat. What do you guys think about an option {} in the filename which would be replaced with sizexsize?

  {
    "folder": "../frontend/src/assets/icons",
    "name": "icon-{}.png", // would be icon-72x72.png for example.
    "sizes": [72, 96, 128, 144, 152, 192, 384, 512]
  }

this looks cleaner & more straightforward to me.

@lorenzolewis
Copy link
Member

One thing I think could be done is to have some smart defaults here that don't require a user to setup a custom json file. For instance, have filenames with mac-512.png, windows-512.png, linux-512.png, etc. could be a useful defaults (but a user could always give alternatives).

@khuongduy354
Copy link
Contributor

Since this issue is in a deprecated repo. I want to clarify it a bit. The relevant file is here.

The goal is to allow custom configs for icons. Something like

yarn tauri icon target-icon.png --ouput icons --config icons/config.json

The configuration for generating icons is currently hardcoded. So, a json format is needed, as I was suggested above:

[
  {
    "folder": "../frontend/src/assets/icons",
    "name": "icon-{}.png", // would be icon-72x72.png for example.
    "sizes": [72, 96, 128, 144, 152, 192, 384, 512]
  }
]

And of course, there'll be default configs if user don't want to customize anything. Is everything all right?

@FabianLars
Copy link
Member

FabianLars commented Sep 18, 2022

As far as i am concerned, the current default output must stay unchanged and this json approach looks fine to me.

That said i didn't really understand Lorenzo's proposal tbh. Also i think there are no other defaults we could/should provide that we don't already have (or will have for new build targets).

Edit: One additional thing: i personally would like to keep it restricted to png for now, if this was in question.

@khuongduy354
Copy link
Contributor

As far as i am concerned, the current default output must stay unchanged and this json approach looks fine to me.

That said i didn't really understand Lorenzo's proposal tbh. Also i think there are no other defaults we could/should provide that we don't already have (or will have for new build targets).

Edit: One additional thing: i personally would like to keep it restricted to png for now, if this was in question.

https://github.com/tauri-apps/tauricon/blob/dev/src/helpers/tauricon.config.ts
do you think the linux defaults above fit ?

@FabianLars
Copy link
Member

Yes, otherwise i would've changed it when i ported it to rust :D

But we couldn't change it anyway, because it would be a weird breaking change, and at least the .deb bundle does use these files currently.

As i said, we can't change anything about tauri icons current output, except for the appx images since they are not used right now. But we decided to just keep them anyway 🤷

@khuongduy354
Copy link
Contributor

khuongduy354 commented Sep 18, 2022

I don't really get it about the default output, you mean the paths or the sizes? Why it can't be changed ? It's hardcoded in the code, can i move it to a json instead ?
image
Edit: Ok, i understand that it'll make a breaking change. So I just move them to json, and keep the number the same, it gonna be fine, right?

@FabianLars
Copy link
Member

FabianLars commented Sep 18, 2022

What i mean with the default output is the files it currently generates if you call tauri icon, both the size and file names (and the path logic i guess). We need it to generate these files like this no matter what.

It's hardcoded in the code, can i move it to a json instead ?

I'd prefer it in the code tbh, but if you can come up with something nice, i'm willing to reconsider.

@khuongduy354
Copy link
Contributor

@FabianLars
I tried using it with serde, it seems good, like the fn icns here. But there're things I'm not sure:

  1. there're 4 sizes covered in fn ico, but only 1 .ico is generated, is it a bug or is it made that way?
    image
  2. in fn png
    image
    Is @2x necessary? Can I make it to 256x256 instead, cuz it will fit the format you suggested easier.

Also, I want to ask a bit about workflow. Should I ask for help, or discuss in Discord or comment here? And how can I effectively share my prototype code for others to review without making a PR? I'm still a newbie, I'd appreciate it if you could guide me through this stage.

@FabianLars
Copy link
Member

there're 4 sizes covered in fn ico, but only 1 .ico is generated, is it a bug or is it made that way?

This is how the .ico format works. These are just "layers" contained in a single file. Here again, all these sizes need to stay, and the order is important too (32 must be before 16 and 24 so it's the first layer)

Is @2x necessary?

Yes, because it's the 128px icon for high dpi mode. I'd recommend not really touching the current code for now and just slap your own stuff on top, we can refactor it later before merging.

Should I ask for help, or discuss in Discord or comment here?

Discord is preferred for live communication aka chatting. Otherwise the general approach (that i like) seems to be to first try to explore the basics of the topic in the issue, and then if you have some kind of PoC implementation, create a draft PR and move the discussion about specifics over there and only report back major things in the issue to not annoy issue subscribers 😅
But in the end it really doesn't matter too much, it's more about what you're comfortable with.

@khuongduy354
Copy link
Contributor

Here is my first PR

luoffei pushed a commit to luoffei/tauri that referenced this issue Dec 29, 2022
… (tauri-apps#5246)

Co-authored-by: Fabian-Lars <fabianlars@fabianlars.de>
Co-authored-by: Lucas Nogueira <lucas@tauri.studio>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants