Renderz commented on PR #20276: URL: https://github.com/apache/superset/pull/20276#issuecomment-1153520294
> While this fixes the missing class, and I appreciate the contribution to resolve that, there are a couple things that are holding me up from approving/merging. > > 1. The `.m-r-10` definition exists in SQL Lab's `main.less` file, so if we were to go global with it, we should remove that in favor of the one you're adding. > 2. It looks like this class is only actually used in a couple places. It'd probably be better to style those two components with Emotion rather than adding more LESS (which we're trying to whittle away at) > 3. To confuse things further... I kind of LIKE the layout of the Before picture better... everything is nicely right aligned. Maybe we should just get rid of this margin once and for all? It sounds like you were solving a code problem more than a cosmetic problem, but let me know if I'm misinterpreting. I found this problem when I was trying to add another Button on the right side after the one showing in the picture, and there was no margin between these two Button which should be there because both Button had '.m-r-10' class. So I raised up this issue. But just as you mentioned, it looks nicely right aligned so maybe all those buttons should be removed from '.m-r-10' class? or use '.m-l-10' instead. And maybe it is another good way to globalize SQL Labs' less file. -- 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]
