michael-s-molina commented on code in PR #20049:
URL: https://github.com/apache/superset/pull/20049#discussion_r872419428
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -125,37 +125,45 @@ test('Should render default props', () => {
delete props.sliceCanEdit;
render(<SliceHeaderControls {...props} />, { useRedux: true });
- userEvent.click(screen.getByRole('menuitem', { name: 'Maximize chart' }));
- userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
- userEvent.click(
- screen.getByRole('menuitem', { name: 'Toggle chart description' }),
- );
- userEvent.click(
- screen.getByRole('menuitem', { name: 'View chart in Explore' }),
- );
- userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
- userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
+ expect(
+ screen.getByRole('menuitem', { name: 'Enter fullscreen' }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: /Force refresh/ }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: 'Show chart description' }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: 'Edit chart' }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('menuitem', { name: 'Download' }),
+ ).toBeInTheDocument();
+ expect(screen.getByRole('menuitem', { name: 'Share' })).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'More Options' }),
).toBeInTheDocument();
expect(screen.getByTestId('NoAnimationDropdown')).toBeInTheDocument();
});
-test('Should "export to CSV"', () => {
+test('Should "export to CSV"', async () => {
const props = createProps();
render(<SliceHeaderControls {...props} />, { useRedux: true });
expect(props.exportCSV).toBeCalledTimes(0);
- userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
+ userEvent.hover(screen.getByText('Download'));
+ userEvent.click(await screen.findByText('Export to .CSV'));
expect(props.exportCSV).toBeCalledTimes(1);
expect(props.exportCSV).toBeCalledWith(371);
});
test('Should not show "Export to CSV" if slice is filter box', () => {
const props = createProps('filter_box');
render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export CSV' })).toBe(null);
+ userEvent.hover(screen.getByText('Download'));
Review Comment:
Here you need to `await` for the test to be valid. It's the same logic as
the test above when you hover over the Download text and waits to see if the
options appear.
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -34,6 +34,7 @@ import CrossFilterScopingModal from
'src/dashboard/components/CrossFilterScoping
import Icons from 'src/components/Icons';
import ModalTrigger from 'src/components/ModalTrigger';
import ViewQueryModal from 'src/explore/components/controls/ViewQueryModal';
+import { FileImageOutlined, FileOutlined } from '@ant-design/icons';
Review Comment:
Shouldn't these come from our `Icons` component?
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -205,18 +211,19 @@ test('Should not show export full CSV if slice is filter
box', () => {
};
const props = createProps('filter_box');
render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
+ userEvent.hover(screen.getByText('Download'));
+ expect(screen.queryByRole('menuitem', { name: 'Export to full .CSV' })).toBe(
Review Comment:
`await` is needed here as well
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -256,7 +258,9 @@ class SliceHeaderControls extends React.PureComponent<
: item}
</div>
));
- const resizeLabel = isFullSize ? t('Minimize chart') : t('Maximize chart');
+ const resizeLabel = isFullSize
Review Comment:
Maybe rename it to `fullscreenLabel`?
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -193,7 +198,8 @@ test('Should not show export full CSV if report is not
table', () => {
};
const props = createProps();
render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
+ userEvent.hover(screen.getByText('Download'));
+ expect(screen.queryByRole('menuitem', { name: 'Export to full .CSV' })).toBe(
Review Comment:
`await` is needed here as well
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -165,23 +173,20 @@ test('Export full CSV is under featureflag', () => {
};
const props = createProps('table');
render(<SliceHeaderControls {...props} />, { useRedux: true });
- expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
- null,
- );
+ userEvent.hover(screen.getByText('Download'));
+ expect(screen.queryByText('Export full CSV')).toBe(null);
Review Comment:
`await` is needed here as well
--
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]
