gabotorresruiz commented on code in PR #37708:
URL: https://github.com/apache/superset/pull/37708#discussion_r2776739264
##########
superset-frontend/src/explore/components/SaveModal.test.jsx:
##########
@@ -25,6 +25,8 @@ import {
render,
waitFor,
within,
+ screen,
+ userEvent,
Review Comment:
Same as above. I think this isn't being used
##########
superset-frontend/src/explore/components/SaveModal.test.jsx:
##########
@@ -25,6 +25,8 @@ import {
render,
waitFor,
within,
+ screen,
Review Comment:
I don’t see this being used anywhere in the existing test suites. Is it
needed?
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -618,11 +619,28 @@ class SaveModal extends Component<SaveModalProps,
SaveModalState> {
/>
</FormItem>
{this.props.datasource?.type === 'query' && (
- <FormItem label={t('Dataset Name')} required>
- <InfoTooltip
- tooltip={t('A reusable dataset will be saved with your chart.')}
- placement="right"
- />
+ <FormItem
+ label={
+ <div>
Review Comment:
I think this is not taking full advantage of the AntD Flex component. We can
simplify this to just:
```jsx
<FormItem
required
label={
<Flex
align="center"
css={theme => css`
gap: ${theme.sizeUnit}px;
`}
>
{t('Dataset Name')}
<InfoTooltip
data-test="info-tooltip-icon"
tooltip={t(
'A reusable dataset will be saved with your chart.',
)}
placement="right"
/>
</Flex>
}
>
```
It could be just:
```jsx
...
label={
<Flex gap={theme.sizeUnit}>
...
</Flex>
...
```
But since this is a class component, `theme` is not directly available in
the render method. The `css` prop callback is the simplest way to access it here
--
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]
