vaetas / heroicons

HeroIcons SVG icons for Flutter
https://pub.dev/packages/heroicons
MIT License
15 stars 10 forks source link

possibility to change version #17

Open EglerEnrique opened 1 year ago

EglerEnrique commented 1 year ago

Is it possible to switch between heroicons v1 and v2 on branch 0.8.0?

vaetas commented 1 year ago

Sorry, I don't think this is currently supported. I haven't been following the development of HeroIcons so I am not familiar with the changes.

However, if somebody is willing to implement this in a PR (if it's a stable feature) I would accept that.

SamJakob commented 1 year ago

I've created #18 to allow switching between different versions (by specifying a branch with the fetch_icons command) but it only works for branches based on v2 for now because the v1 branch has a different directory structure and icon sets available. I'm not sure exactly what the use case is for v1 so I haven't altered the directory structure or code of the package itself to support using v1 icons yet.

@EglerEnrique - I'm curious as to why you might need this? It seems we've been on v2 icons for some time now, and v2 seems to include all the icons of v1, so do you need to be able to switch between these icons within your app (e.g., at runtime) or is it just a matter of having a branch available with v1 icons?

My reservation about simply adding them alongside the v2 icons in the main branch is that it would presumably increase the size of the application given that all of the v1 assets would need to be included as well.

That said, it seems the size of the old assets is ~2MB in total and the current one is ~3.5MB in total -- I'm not sure how precious people are about reducing app size, maybe nobody cares in 2023 so 5MB is not a concern, in which case I guess it's worth including v1 icons too.

(Also the specific need for v1 icons could affect the way the API for this could be implemented - e.g., with an additional set of classes, a 'v1' style, or 'v1'-specific icons in addition to the ones already there [such as having academicCap and v1AcademicCap]).

vaetas commented 1 year ago

Thank you all for discussion @SamJakob @EglerEnrique

Unfortunately I have a lot of work right now and I'm not able to keep track of development of HeroIcons.

I lean heavily against including multiple versions of the icons into the package. Size is one major problem and the usage is the second. (I don't think adding new constructor with prefixes like v1 brings any value.)

I mostly prefer using the most recent production version of the icons. If people need different branch they are free to fork this package and customize it. I don't think majority of people would benefit from this feature and it would only negatively impact them (i.e. app size).

@SamJakob Thank you very much for the PR! I assume that with your changes I could only pull the most recent version, right? I don't mind keeping the option there. People that fork the repo could easily pull different branch. But the pub.dev version would always be strictly the one most upstream (production) version. Would that be ok with you?

SamJakob commented 1 year ago

@vaetas Yeah that's right! It'll pull latest by default.

SamJakob commented 1 year ago

And yes - I agree. I think I'll maybe adapt the scripts at some point - if there's any form of interest in v1 icons - and this will allow people to easily fork the package and generate the code for v1 icons.

For now, I 100% agree that this should just track the latest version. Could always add a branch on this package for v1 as it shouldn't change and this should give some flexibility with dealing with versions in future - say if heroicons ever releases a v3.