EnxDev commented on code in PR #34067:
URL: https://github.com/apache/superset/pull/34067#discussion_r2194801626
##########
superset-frontend/src/pages/ChartList/index.tsx:
##########
@@ -774,10 +774,17 @@ function ChartList(props: ChartListProps) {
if (canCreate) {
subMenuButtons.push({
name: (
- <>
+ <span
Review Comment:
If we want, we can keep the current implementation by simply renaming the
name prop (I agree with you — it was a bit confusing at first). When there’s no
tooltip, we can use the icon prop to render the icons.
Regarding the icon alignment relative to the text, there's a slight mismatch
due to the different line-height. We can leave it as is, unless we want to
apply additional styles like display: flex, which could break some vertical
alignments. In that case, we could preserve the current font-size and
line-height.
If it’s not already supported for the button, I think we could consider
introducing props like prependIcon and appendIcon.
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
