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: [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]
