UI icon size #6736
UI icon size #6736
Conversation
|
Hey 👋 Only had a quick glance at this, but my immediate thought was - how come you decided you wanted to move away from Would be good to understand some of the reasoning behind this! Thanks, John |
|
Hey @johncowen ! Yeah there's a bit of a story there :D So The motivation here, moving from classes specific to the component all over the application to a property on the component, is really just one of encapsulation. The sizing classes are implementation details and it felt like having to use them for sizing all over the application was leaking those details. If the sizing classes are encapsulated in the component itself, they can be changed in one spot if needed at a later date. This encapsulation also keeps the interface / api surface of the component in JS, not a combination of JS and CSS which is a tidier boundary. If an application needs to do additional styling with it, there's still that option via CSS, but it's then an application concern, not a component one. |
There was a problem hiding this comment.
Oki doki, just seems strange to me that I set sizes of things for everything else using a class.
Also, on this component if I want to change the size I use the size attribute, but if I want to change the color I use class=""?
Most importantly though it needs to feel right to you, so as long as you are happy with it, it's all good with me!
Code looks fine, I had a comment, but then went 'oh yeah, I see now!', but left the comment in just to see if I guessed right
|
|
||
| storiesOf('Icon/', module) | ||
| .addParameters({ options: { showPanel: false } }) | ||
| .addParameters({ options: { showPanel: true} }) |
There was a problem hiding this comment.
Not sure if you left this in by accident (showPanel: true)? Ah, maybe you want the panel in now as you have a setting you can tweak.? Will leave this comment in just incase, but 99% sure this is good
There was a problem hiding this comment.
Yeah, this is so that the knobs panel shows up when you click Icon in the sidebar, otherwise it's hidden. We usually hide it if we're not using the knobs decorator, but since I added it here, I flipped it to true.
* master: (85 commits) UI icon - add size (#6736) Add Priority Queue library to sdk (#6664) Add spellcheck="false" to form fields (#6744) Maximum typo in Vault UI (#6743) docs: Fix Markdown formatting error in AWS Auth (#6745) changelog++ Update OIDC Provider Setup docs (#6739) Update to use newer sdk Copy LogInput from audit package, add OptMarshaler interface (#6735) docs: fixed typo (#6732) Fix typo changelog++ Use Go modules in CircleCI (#6729) Fix recovery key backup path documentation Vendoring updated grpc Add link to R client on libraries list (#6722) UI ember engines (#6718) changelog++ Update grpc and protos (#6725) changelog++ ...
Adds a
sizeproperty to the newIconcomponent so that we don't have to use the component's CSS classes for sizing.