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

Option for size-specific icons #4950

Closed
fabric-8 opened this issue Feb 16, 2024 · 4 comments · Fixed by #4957
Closed

Option for size-specific icons #4950

fabric-8 opened this issue Feb 16, 2024 · 4 comments · Fixed by #4957
Assignees
Labels
area:ui UI engineering specific tasks.

Comments

@fabric-8
Copy link
Contributor

If I understand the new icon implementation correctly we're always just using the default 24x24 asset/svg and scaling it when using them at different sizes within the UI. This approach "breaks" the stroke width which isn't just scaled down from 24x24 and in some cases we're actually using different assets for 24x24 vs. 16x16.

Suggestion:

  • Optionally be able to provide svg code for multiple sizes within the icon file itself
  • If no specific svg code for other sizes is provided, fall back to 24x24 asset and scale it (ideally, with a set stroke width of 1.5 for 16x16 / 2.5 for 32x32 though)

Happy to collaborate on this and provide assets where necessary!

@fabric-8 fabric-8 added this to the Establish UI library milestone Feb 16, 2024
@fabric-8 fabric-8 added area:ui UI engineering specific tasks. needs:dev-input labels Feb 16, 2024
@fbwoolf
Copy link
Contributor

fbwoolf commented Feb 16, 2024

@fabric-8 ok, I didn't understand that from the original issue description. Should we just always provide what we need rather than having fallbacks? I'd prefer to avoid needing conditionals and things just for a simple icon. From other references, is it typ to need so much code arnd icons? This is the ref storybook I used, and it simply has a size prop in the code and does just adjust the height/width of one svg:

https://main--64b56e737c0aeefed9d5e675.chromatic.com/?path=/docs/introduction--docs
https://github.com/storybookjs/icons

@fabric-8
Copy link
Contributor Author

Sorry, I have spoken about this with @kyranjamie earlier, should have been more explicit about this in the issue.

The implementation was just an idea from my end I imagined to be a nice approach, we can def. leave out the entire fallback/conditional part. But being able to have size-specific svg code would be great. If that's too much effort for now let me know and I'll try to adjust the icons in question in a way that makes them scale / work a bit better.

Straight-up scaling is a bit gnarly as the stroke width values you'll end up with are going to be quite random and off-(pixel)grid

@fbwoolf
Copy link
Contributor

fbwoolf commented Feb 16, 2024

Straight-up scaling is a bit gnarly as the stroke width values you'll end up with are going to be quite random and off-(pixel)grid

Ok, np, I'll move fwd and refactor it with your requests here. I didn't realize it was discussed already! Sorry for not doing it correctly the first time through. Should be quicker now though with the new icons setup already. 👍

@fabric-8
Copy link
Contributor Author

Awesome! If you need a helping hand doing the grunt work and getting the svg code for the 16 versions in please let me know, happy to help out!

kyranjamie pushed a commit that referenced this issue Feb 26, 2024
## [6.28.0](v6.27.2...v6.28.0) (2024-02-26)

### Features

* add dark splash screen, ref [#4398](#4398) ([c4fb072](c4fb072))
* check utxo ids for inscriptions, ref [#4920](#4920) ([86dd00d](86dd00d))

### Bug Fixes

* attempt to fix failing test ([0ff7701](0ff7701))
* broken color on welcome page ([384c947](384c947))
* memo using old input ([8829a2a](8829a2a))
* remove hiro ref in page ([c49f7e9](c49f7e9))
* stamps api, closes [#4845](#4845) ([3230c49](3230c49))

### Internal

* additional checks before adding tapInternalKey, ref [#4125](#4125) ([09a17bf](09a17bf))
* colors, closes [#4831](#4831) ([934cfd0](934cfd0))
* esm webpack ([3cc8878](3cc8878))
* existing icons to use variants, closes [#4950](#4950) ([2ce7319](2ce7319))
* icon variants ([7a886c1](7a886c1))
* **input:** remove InputTextField component ([3556390](3556390))
* new icons using icon gallery ([dc1bf28](dc1bf28))
* post-release merge back ([5a87a5d](5a87a5d))
* upgrade packages ([dcc36d0](dcc36d0))
pete-watters pushed a commit that referenced this issue Mar 4, 2024
## [6.28.0](v6.27.2...v6.28.0) (2024-02-26)

### Features

* add dark splash screen, ref [#4398](#4398) ([c4fb072](c4fb072))
* check utxo ids for inscriptions, ref [#4920](#4920) ([86dd00d](86dd00d))

### Bug Fixes

* attempt to fix failing test ([0ff7701](0ff7701))
* broken color on welcome page ([384c947](384c947))
* memo using old input ([8829a2a](8829a2a))
* remove hiro ref in page ([c49f7e9](c49f7e9))
* stamps api, closes [#4845](#4845) ([3230c49](3230c49))

### Internal

* additional checks before adding tapInternalKey, ref [#4125](#4125) ([09a17bf](09a17bf))
* colors, closes [#4831](#4831) ([934cfd0](934cfd0))
* esm webpack ([3cc8878](3cc8878))
* existing icons to use variants, closes [#4950](#4950) ([2ce7319](2ce7319))
* icon variants ([7a886c1](7a886c1))
* **input:** remove InputTextField component ([3556390](3556390))
* new icons using icon gallery ([dc1bf28](dc1bf28))
* post-release merge back ([5a87a5d](5a87a5d))
* upgrade packages ([dcc36d0](dcc36d0))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ui UI engineering specific tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants