Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: render without routing when using as a React component #838

Merged
merged 8 commits into from
May 5, 2022
Merged

Fix: render without routing when using as a React component #838

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
58fac9f
render the react component without routing
tynandebold May 3, 2022
6a04f8c
add a test
tynandebold May 3, 2022
739b6f2
update test
tynandebold May 4, 2022
4b4bcf7
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fi…
tynandebold May 5, 2022
e092896
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fi…
tynandebold May 5, 2022
85a5218
update test
tynandebold May 5, 2022
00c18ec
update test
tynandebold May 5, 2022
f9db08e
Merge branch 'main' into fix/standlone-component-routing
tynandebold May 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/components/wrapper/wrapper.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Wrapper, mapStateToProps } from './wrapper';
import { mockState, setup } from '../../utils/state.mock';
import { MemoryRouter } from 'react-router-dom';

const { theme } = mockState.spaceflights;
const mockProps = {
Expand Down Expand Up @@ -32,9 +33,15 @@ describe('Wrapper', () => {
expect(container.length).toBe(0);
});

it('does not display the settings modal when displayGlobalToolbar is false', () => {
const wrapper = setup.mount(Wrapper, mockPropsNoGlobalToolbar);
const container = wrapper.find('.pipeline-settings-modal');
it('does not perform any routing when displayGlobalToolbar is false', () => {
const wrapper = setup.mount(
<MemoryRouter initialEntries={['/experiment-tracking']}>
<Wrapper />
</MemoryRouter>,
mockPropsNoGlobalToolbar
);

const container = wrapper.find('.pipeline-ui--experiment-tracking');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this test - this is definitely structured in a much better way.

I've just had a play around with this and tried testing this using mockProps instead of mockPropsNoGlobalToolbar but unfortunately this still passes, which indicates that there is something missing from this setup.

This could either be that we are testing with the wrong css class, or something to do with the setup - worth to do a bit of digging around to find out and set up the right test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling on this one. I don't see a good way to test this.

Since we'll only render the FlowChartWrapper when we use mockPropsNoGlobalToolbar, we won't have access to any routing, so actually my use of MemoryRouter doesn't make sense here.

And since we don't have access to that routing or even react-router at this stage, I don't see a way to mock navigation to one of the routes using any of the methods you normally would use when routing is present on the page or in the component.

If you have a better idea, do please share it, because I can't see it.

expect(container.length).toBe(0);
});

Expand Down