mistercrunch commented on PR #34067:
URL: https://github.com/apache/superset/pull/34067#issuecomment-3037061238

   Mmmh, does antd gets this wrong, or is it because we're not using antd right?
   
   I noticed antd's button expects an `icon` prop, but often we pass it as a 
children instead, is that what we're addressing here?
   
   If it's the case I'd much rather fix our use of `<Button icon={} />` 
throughout the codebase than patching it with styles in the wrapper.
   
   From my understanding, if/when using button with the icon prop, antd should 
do all the right things in terms of alignement with standard sizing and all. 
Guessing it may pass-in/override the icon sizing based on the button's size 
attributes, also probably should get the color of the icon to match the text 
and all. I've seen places where we `theme.colors.warning.light4` on the icon to 
try and match the text's color and that's just stuff antd will do properly when 
used right ....


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to