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

Show "Custom" section in "Get Started" screen #3523

Merged
merged 15 commits into from
Apr 3, 2023
Merged

Show "Custom" section in "Get Started" screen #3523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
883737d
Show custom plots in Get Started screen
julieg18 Mar 21, 2023
e1c7dbf
Merge branch 'main' into show-custom-plots-data-in-get-started
julieg18 Mar 24, 2023
222b9b6
Fix bugs
julieg18 Mar 25, 2023
547fbcb
Remove ribbon
julieg18 Mar 27, 2023
fddccd0
Fix typo
julieg18 Mar 27, 2023
d498e0c
Cleanup
julieg18 Mar 27, 2023
5619a0c
Merge branch 'main' into show-custom-plots-data-in-get-started
julieg18 Mar 27, 2023
7c90fbe
Merge branch 'main' into show-custom-plots-data-in-get-started
julieg18 Mar 28, 2023
6ed0863
Use message in section instead of empty plots
julieg18 Mar 28, 2023
8ca7c73
Remove trends logic
julieg18 Mar 28, 2023
3bf96bd
Only show custom section when there are plots
julieg18 Mar 31, 2023
7374c2e
Merge branch 'main' into show-custom-plots-data-in-get-started
mattseddon Apr 3, 2023
771b9e0
Use only one text sentence
julieg18 Apr 3, 2023
a2763b4
Merge branch 'main' into show-custom-plots-data-in-get-started
julieg18 Apr 3, 2023
39b349a
Merge branch 'main' into show-custom-plots-data-in-get-started
julieg18 Apr 3, 2023
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
147 changes: 99 additions & 48 deletions webview/src/plots/components/App.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,17 +318,20 @@ describe('App', () => {
expect(loading).toHaveLength(3)
})

it('should render the Add Plots and Add Experiments get started button when there are experiments which have plots that are all unselected', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried my best to cover all the possibilities for the get started screen and to make the rather lengthy text clear 😅

it('should render only get started (buttons: add plots, add experiments, add custom plots) when there are some selected exps, all unselected plots, and no custom plots', async () => {
renderAppWithOptionalData({
hasPlots: true,
hasUnselectedPlots: true,
selectedRevisions: [{} as Revision]
})
const addExperimentsButton = await screen.findByText('Add Experiments')
const addPlotsButton = await screen.findByText('Add Plots')
const addCustomPlotsButton = await screen.findByText('Add Custom Plot')

expect(addExperimentsButton).toBeInTheDocument()
expect(addPlotsButton).toBeInTheDocument()
expect(addCustomPlotsButton).toBeInTheDocument()
expect(screen.queryByTestId('section-container')).not.toBeInTheDocument()

mockPostMessage.mockReset()

Expand All @@ -346,9 +349,51 @@ describe('App', () => {
type: MessageFromWebviewType.SELECT_PLOTS
})
mockPostMessage.mockReset()

fireEvent.click(addCustomPlotsButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.ADD_CUSTOM_PLOT
})
mockPostMessage.mockReset()
})

it('should render only the Add Experiments get started button when no experiments are selected', async () => {
it('should render get started (buttons: add plots, add experiments) and custom section when there are some selected exps, all unselected plots, and added custom plots', async () => {
renderAppWithOptionalData({
custom: customPlotsFixture,
hasPlots: true,
hasUnselectedPlots: true,
selectedRevisions: [{} as Revision]
})
const addExperimentsButton = await screen.findByText('Add Experiments')
const addPlotsButton = await screen.findByText('Add Plots')
const addCustomPlotsButton = screen.queryByText('Add Custom Plot')
const customSection = await screen.findByTestId('section-container')

expect(addExperimentsButton).toBeInTheDocument()
expect(addPlotsButton).toBeInTheDocument()
expect(addCustomPlotsButton).not.toBeInTheDocument()
expect(customSection).toBeInTheDocument()

mockPostMessage.mockReset()

fireEvent.click(addExperimentsButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SELECT_EXPERIMENTS
})

mockPostMessage.mockReset()

fireEvent.click(addPlotsButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SELECT_PLOTS
})
mockPostMessage.mockReset()
})

it('should render only get started (buttons: add experiments, add custom plots) when there are no selected exps and no custom plots', async () => {
renderAppWithOptionalData({
custom: null,
hasPlots: true,
Expand All @@ -357,32 +402,56 @@ describe('App', () => {
})
const addExperimentsButton = await screen.findByText('Add Experiments')
const addPlotsButton = screen.queryByText('Add Plots')
const addCustomPlotsButton = await screen.findByText('Add Custom Plot')
const customSection = screen.queryByTestId('section-container')

expect(addExperimentsButton).toBeInTheDocument()
expect(addCustomPlotsButton).toBeInTheDocument()
expect(addPlotsButton).not.toBeInTheDocument()
expect(customSection).not.toBeInTheDocument()

mockPostMessage.mockReset()

fireEvent.click(addExperimentsButton)
expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SELECT_EXPERIMENTS
})

fireEvent.click(addCustomPlotsButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.ADD_CUSTOM_PLOT
})
mockPostMessage.mockReset()
})

it('should render an empty state given a message with only custom plots data', () => {
it('should render get started (buttons: add experiments) and custom section when there are no selected exps and added custom plots', async () => {
renderAppWithOptionalData({
custom: customPlotsFixture
custom: customPlotsFixture,
hasPlots: true,
hasUnselectedPlots: false,
selectedRevisions: undefined
})

expect(screen.queryByText('Loading Plots...')).not.toBeInTheDocument()
const addExperimentsButton = screen.queryByText('Add Experiments')
const addExperimentsButton = await screen.findByText('Add Experiments')
const addPlotsButton = screen.queryByText('Add Plots')
const addCustomPlotsButton = screen.queryByText('Add Custom Plot')
const customSection = await screen.findByTestId('section-container')

expect(addExperimentsButton).toBeInTheDocument()
expect(addCustomPlotsButton).not.toBeInTheDocument()
expect(addPlotsButton).not.toBeInTheDocument()
expect(customSection).toBeInTheDocument()

mockPostMessage.mockReset()

fireEvent.click(addExperimentsButton)
expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SELECT_EXPERIMENTS
})
})

it('should render custom with "No Plots to Display" message when there is no custom plots data', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
template: templatePlotsFixture
})

Expand All @@ -393,16 +462,16 @@ describe('App', () => {

it('should render custom with "No Plots Added" message when there are no plots added', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: {
...customPlotsFixture,
plots: []
}
},
template: templatePlotsFixture
})

expect(screen.queryByText('Loading Plots...')).not.toBeInTheDocument()
expect(screen.queryByText('No Plots to Display')).not.toBeInTheDocument()
expect(screen.getByText('Custom')).toBeInTheDocument()
expect(screen.getByText('No Plots to Display')).toBeInTheDocument()
expect(screen.getByText('No Plots Added')).toBeInTheDocument()
})

Expand Down Expand Up @@ -439,24 +508,18 @@ describe('App', () => {
expect(emptyState).toBeInTheDocument()
})

it('should remove custom plots given a message showing custom plots as null', async () => {
const emptyStateText = 'No Plots to Display'

it('should remove custom plots given a message showing custom plots as null', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture,
template: templatePlotsFixture
custom: customPlotsFixture
})

expect(screen.queryByText(emptyStateText)).not.toBeInTheDocument()
expect(screen.getByText('Custom')).toBeInTheDocument()

sendSetDataMessage({
custom: null
})

const emptyState = await screen.findByText(emptyStateText)

expect(emptyState).toBeInTheDocument()
expect(screen.queryByText('Custom')).not.toBeInTheDocument()
})

it('should remove all sections from the document if there is no data provided', () => {
Expand All @@ -475,7 +538,6 @@ describe('App', () => {

it('should toggle the custom plots section in state when its header is clicked', async () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})

Expand Down Expand Up @@ -510,7 +572,6 @@ describe('App', () => {

it('should not toggle the custom plots section when its header is clicked and its title is selected', async () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})

Expand Down Expand Up @@ -577,7 +638,6 @@ describe('App', () => {

it('should not toggle the custom plots section when its header is clicked and the content of its tooltip is selected', async () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})

Expand Down Expand Up @@ -632,12 +692,11 @@ describe('App', () => {

it('should display a slider to pick the number of items per row if there are items and the action is available', () => {
const store = renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})
setWrapperSize(store)

expect(screen.getAllByTestId('size-sliders')[1]).toBeInTheDocument()
expect(screen.getByTestId('size-sliders')).toBeInTheDocument()
})

it('should not display a slider to pick the number of items per row if there are no items', () => {
Expand All @@ -649,7 +708,6 @@ describe('App', () => {

it('should not display a slider to pick the number of items per row if the only width available for one item per row or less', () => {
const store = renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})
setWrapperSize(store, 400)
Expand All @@ -672,13 +730,12 @@ describe('App', () => {

it('should display both size sliders for custom plots', () => {
const store = renderAppWithOptionalData({
custom: customPlotsFixture,
template: templatePlotsFixture
custom: customPlotsFixture
})
setWrapperSize(store)

const plotResizers = within(
screen.getAllByTestId('size-sliders')[1]
screen.getByTestId('size-sliders')
).getAllByRole('slider')

expect(plotResizers.length).toBe(2)
Expand All @@ -699,14 +756,13 @@ describe('App', () => {

it('should send a message to the extension with the selected size when changing the width of plots', () => {
const store = renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})
setWrapperSize(store)

const plotResizer = within(
screen.getAllByTestId('size-sliders')[1]
).getAllByRole('slider')[0]
const plotResizer = within(screen.getByTestId('size-sliders')).getAllByRole(
'slider'
)[0]

fireEvent.change(plotResizer, { target: { value: -3 } })
expect(mockPostMessage).toHaveBeenCalledWith({
Expand All @@ -721,14 +777,13 @@ describe('App', () => {

it('should send a message to the extension with the selected size when changing the height of plots', () => {
const store = renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})
setWrapperSize(store)

const plotResizer = within(
screen.getAllByTestId('size-sliders')[1]
).getAllByRole('slider')[1]
const plotResizer = within(screen.getByTestId('size-sliders')).getAllByRole(
'slider'
)[1]

fireEvent.change(plotResizer, { target: { value: 3 } })
expect(mockPostMessage).toHaveBeenCalledWith({
Expand Down Expand Up @@ -770,7 +825,6 @@ describe('App', () => {

it('should send a message to the extension when the custom plots are reordered', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})

Expand Down Expand Up @@ -805,7 +859,6 @@ describe('App', () => {

it('should add a custom plot if a user creates a custom plot', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: {
...customPlotsFixture,
plots: customPlotsFixture.plots.slice(0, 3)
Expand Down Expand Up @@ -836,7 +889,6 @@ describe('App', () => {

it('should remove a custom plot if a user deletes a custom plot', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
})

Expand Down Expand Up @@ -1350,7 +1402,6 @@ describe('App', () => {

it('should open a modal with the plot zoomed in when clicking a custom plot', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
custom: customPlotsFixture
julieg18 marked this conversation as resolved.
Show resolved Hide resolved
})

Expand Down Expand Up @@ -1494,7 +1545,7 @@ describe('App', () => {
describe('Large plots', () => {
it('should wrap the custom plots in a big grid (virtualize them) when there are more than eight large plots', async () => {
await renderAppAndChangeSize(
{ comparison: comparisonTableFixture, custom: createCustomPlots(9) },
{ custom: createCustomPlots(9) },
1,
PlotsSection.CUSTOM_PLOTS
)
Expand All @@ -1512,7 +1563,7 @@ describe('App', () => {

it('should not wrap the custom plots in a big grid (virtualize them) when there are eight or fewer large plots', async () => {
await renderAppAndChangeSize(
{ comparison: comparisonTableFixture, custom: createCustomPlots(8) },
{ custom: createCustomPlots(8) },
1,
PlotsSection.CUSTOM_PLOTS
)
Expand Down Expand Up @@ -1621,7 +1672,7 @@ describe('App', () => {
describe('Regular plots', () => {
it('should wrap the custom plots in a big grid (virtualize them) when there are more than fourteen regular plots', async () => {
await renderAppAndChangeSize(
{ comparison: comparisonTableFixture, custom: createCustomPlots(15) },
{ custom: createCustomPlots(15) },
DEFAULT_NB_ITEMS_PER_ROW,
PlotsSection.CUSTOM_PLOTS
)
Expand All @@ -1631,7 +1682,7 @@ describe('App', () => {

it('should not wrap the custom plots in a big grid (virtualize them) when there are fourteen regular plots', async () => {
await renderAppAndChangeSize(
{ comparison: comparisonTableFixture, custom: createCustomPlots(14) },
{ custom: createCustomPlots(14) },
DEFAULT_NB_ITEMS_PER_ROW,
PlotsSection.CUSTOM_PLOTS
)
Expand Down Expand Up @@ -1718,7 +1769,7 @@ describe('App', () => {
describe('Smaller plots', () => {
it('should wrap the custom plots in a big grid (virtualize them) when there are more than twenty small plots', async () => {
await renderAppAndChangeSize(
{ comparison: comparisonTableFixture, custom: createCustomPlots(21) },
{ custom: createCustomPlots(21) },
4,
PlotsSection.CUSTOM_PLOTS
)
Expand All @@ -1728,7 +1779,7 @@ describe('App', () => {

it('should not wrap the custom plots in a big grid (virtualize them) when there are twenty or fewer small plots', async () => {
await renderAppAndChangeSize(
{ comparison: comparisonTableFixture, custom: createCustomPlots(20) },
{ custom: createCustomPlots(20) },
4,
PlotsSection.CUSTOM_PLOTS
)
Expand Down
Loading