diff --git a/webview/src/plots/components/App.test.tsx b/webview/src/plots/components/App.test.tsx index 8c7a56dd13..1fd5997b1a 100644 --- a/webview/src/plots/components/App.test.tsx +++ b/webview/src/plots/components/App.test.tsx @@ -318,7 +318,7 @@ 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 () => { + 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, @@ -326,9 +326,12 @@ describe('App', () => { }) 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() @@ -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, @@ -357,9 +402,13 @@ 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() @@ -367,22 +416,42 @@ describe('App', () => { 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 }) @@ -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() }) @@ -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', () => { @@ -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 }) @@ -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 }) @@ -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 }) @@ -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', () => { @@ -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) @@ -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) @@ -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({ @@ -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({ @@ -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 }) @@ -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) @@ -836,7 +889,6 @@ describe('App', () => { it('should remove a custom plot if a user deletes a custom plot', () => { renderAppWithOptionalData({ - comparison: comparisonTableFixture, custom: customPlotsFixture }) @@ -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 }) @@ -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 ) @@ -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 ) @@ -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 ) @@ -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 ) @@ -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 ) @@ -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 ) diff --git a/webview/src/plots/components/GetStarted.tsx b/webview/src/plots/components/GetStarted.tsx index 6b5da73fed..db44995ac0 100644 --- a/webview/src/plots/components/GetStarted.tsx +++ b/webview/src/plots/components/GetStarted.tsx @@ -5,13 +5,15 @@ import { StartButton } from '../../shared/components/button/StartButton' export type AddPlotsProps = { hasUnselectedPlots: boolean + hasNoCustomPlots: boolean } export const AddPlots: React.FC = ({ - hasUnselectedPlots + hasUnselectedPlots, + hasNoCustomPlots }: AddPlotsProps) => (
-

No Plots to Display.

+

No Plots to Display

@@ -33,6 +35,18 @@ export const AddPlots: React.FC = ({ text="Add Plots" /> )} + {hasNoCustomPlots && ( + + sendMessage({ + type: MessageFromWebviewType.ADD_CUSTOM_PLOT + }) + } + text="Add Custom Plot" + /> + )}
) diff --git a/webview/src/plots/components/Plots.tsx b/webview/src/plots/components/Plots.tsx index 994d3c9a87..5ded53adba 100644 --- a/webview/src/plots/components/Plots.tsx +++ b/webview/src/plots/components/Plots.tsx @@ -7,6 +7,7 @@ import { TemplatePlotsWrapper } from './templatePlots/TemplatePlotsWrapper' import { ComparisonTableWrapper } from './comparisonTable/ComparisonTableWrapper' import { Ribbon } from './ribbon/Ribbon' import { setMaxNbPlotsPerRow, setZoomedInPlot } from './webviewSlice' +import styles from './styles.module.scss' import { EmptyState } from '../../shared/components/emptyState/EmptyState' import { Modal } from '../../shared/components/modal/Modal' import { WebviewWrapper } from '../../shared/components/webviewWrapper/WebviewWrapper' @@ -24,6 +25,9 @@ const PlotsContent = () => { const hasTemplateData = useSelector( (state: PlotsState) => state.template.hasData ) + const customPlotIds = useSelector( + (state: PlotsState) => state.custom.plotsIds + ) const wrapperRef = createRef() useLayoutEffect(() => { @@ -47,13 +51,39 @@ const PlotsContent = () => { return Loading Plots... } + const modal = zoomedInPlot?.plot && ( + { + dispatch(setZoomedInPlot(undefined)) + }} + > + + + ) + + const hasNoCustomPlots = customPlotIds.length === 0 + if (!hasComparisonData && !hasTemplateData) { return ( - } - showEmpty={!hasPlots} - welcome={} - /> +
+ + } + showEmpty={!hasPlots} + welcome={} + isFullScreen={hasNoCustomPlots} + /> + {!hasNoCustomPlots && ( + <> + + {modal} + + )} +
) } @@ -64,15 +94,7 @@ const PlotsContent = () => { - {zoomedInPlot?.plot && ( - { - dispatch(setZoomedInPlot(undefined)) - }} - > - - - )} + {modal} ) } diff --git a/webview/src/plots/components/ribbon/Ribbon.tsx b/webview/src/plots/components/ribbon/Ribbon.tsx index 8b15df69ae..4691a65716 100644 --- a/webview/src/plots/components/ribbon/Ribbon.tsx +++ b/webview/src/plots/components/ribbon/Ribbon.tsx @@ -32,9 +32,18 @@ export const Ribbon: React.FC = () => { [dispatch] ) + const setInitialRibbonHeight = useCallback( + () => dispatch(update(0)), + [dispatch] + ) + useEffect(() => { changeRibbonHeight() - }, [revisions, changeRibbonHeight]) + + return () => { + setInitialRibbonHeight() + } + }, [revisions, changeRibbonHeight, setInitialRibbonHeight]) useEffect(() => { window.addEventListener('resize', changeRibbonHeight) diff --git a/webview/src/plots/components/ribbon/ribbonSlice.ts b/webview/src/plots/components/ribbon/ribbonSlice.ts index 4e035d5072..108750b5f6 100644 --- a/webview/src/plots/components/ribbon/ribbonSlice.ts +++ b/webview/src/plots/components/ribbon/ribbonSlice.ts @@ -5,7 +5,7 @@ export interface RibbonState { } export const ribbonInitialState: RibbonState = { - height: 50 + height: 0 } export const ribbonSlice = createSlice({ diff --git a/webview/src/plots/components/styles.module.scss b/webview/src/plots/components/styles.module.scss index c081216a94..1236f83abc 100644 --- a/webview/src/plots/components/styles.module.scss +++ b/webview/src/plots/components/styles.module.scss @@ -3,6 +3,17 @@ $gap: 20px; +.getStartedWrapper { + display: flex; + flex-direction: column; + height: 100vh; + width: 100%; + + > *:first-child { + flex-grow: 1; + } +} + .plots { width: 100%; height: 100%; diff --git a/webview/src/shared/components/getStarted/GetStarted.tsx b/webview/src/shared/components/getStarted/GetStarted.tsx index 90257cc9cb..963eaabcc7 100644 --- a/webview/src/shared/components/getStarted/GetStarted.tsx +++ b/webview/src/shared/components/getStarted/GetStarted.tsx @@ -5,16 +5,18 @@ export type GetStartedProps = { addItems: React.ReactNode showEmpty: boolean welcome: React.ReactNode + isFullScreen?: boolean } export const GetStarted: React.FC = ({ addItems, welcome, - showEmpty + showEmpty, + isFullScreen }: GetStartedProps) => { if (!showEmpty) { - return {addItems} + return {addItems} } - return {welcome} + return {welcome} }