[GitHub] [superset] geido commented on a diff in pull request #19558: feat(explore): Redesign of Run/Save buttons

[GitBox <gi...@apache.org>]

geido commented on code in PR #19558:
URL: https://github.com/apache/superset/pull/19558#discussion_r847422405


##########
superset-frontend/src/explore/components/ExploreChartHeader/index.jsx:
##########
@@ -231,11 +231,13 @@ export class ExploreChartHeader extends React.PureComponent {
       isStarred,
       sliceUpdated,
       sliceName,
+      onSaveChart,
+      saveDisabled,
     } = this.props;
     const { latestQueryFormData, sliceFormData } = chart;
     const oldSliceName = slice?.slice_name;
     return (
-      <StyledHeader id="slice-header">
+      <div id="slice-header" css={headerStyles}>

Review Comment:
   Same comment as the other one for the usage of `css`



##########
superset-frontend/src/explore/components/ControlPanelsContainer.tsx:
##########
@@ -475,6 +538,17 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
           </Tabs.TabPane>
         )}
       </ControlPanelsTabs>
+      <div css={actionButtonsContainerStyles}>

Review Comment:
   A super nit but I think this conflicts a little bit with the Emotion guidelines that we have. The goal of using `css` is to have it closer to the component where it's applied, especially if it's for a single use. In this case, you are using `css` but moving it far from the component to avoid taking too much space here I guess, so probably we should go with the standard `styled.div` instead



-- 
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-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org