Refactor

Signed-off-by: Jitendra Gundaniya <[email protected]>

src/actions/pipelines.js

@@ -98,8 +98,7 @@ export function loadInitialPipelineData() {
     const url = getUrl('main');
     // obtain the status of expandAllPipelines to decide whether it needs to overwrite the
     // list of visible nodes
-    const expandAllPipelines =
-      state.display.expandAllPipelines || state.expandAllPipelines;
+    const expandAllPipelines = state.expandAllPipelines;
     let newState = await loadJsonData(url).then((data) =>
       preparePipelineState(data, true, expandAllPipelines)
     );
@@ -136,8 +135,7 @@ export function loadPipelineData(pipelineID) {
         active: pipelineID,
       });
 
-      const shouldExpandAllPipelines =
-        display.expandAllPipelines || expandAllPipelines;
+      const shouldExpandAllPipelines = expandAllPipelines;
       const newState = await loadJsonData(url).then((data) =>
         preparePipelineState(data, false, shouldExpandAllPipelines)
       );

src/components/app/app.js

@@ -90,25 +90,19 @@ App.propTypes = {
    * If set, this will override the localStorage value.
    */
   theme: PropTypes.oneOf(['dark', 'light']),
-  /**
-   * Override visibility of various features, e.g. icon buttons
-   */
-  visible: PropTypes.shape({
-    labelBtn: PropTypes.bool,
-    layerBtn: PropTypes.bool,
-    exportBtn: PropTypes.bool,
-    expandPipelinesBtn: PropTypes.bool,
-    sidebar: PropTypes.bool,
-  }),
   /**
    * Determines if certain elements are displayed, e.g global tool bar, sidebar
    */
   display: PropTypes.shape({
     globalToolbar: PropTypes.bool,
     sidebar: PropTypes.bool,
     miniMap: PropTypes.bool,
-    expandAllPipelines: PropTypes.bool,
+    expandPipelinesBtn: PropTypes.bool,
+    exportBtn: PropTypes.bool,
+    labelBtn: PropTypes.bool,
+    layerBtn: PropTypes.bool,
   }),
+  expandAllPipelines: PropTypes.bool,
 };
 
 export default App;

src/components/export-modal/export-modal.js

@@ -9,8 +9,8 @@ import './export-modal.scss';
 /**
  * Modal to allow users to choose between SVG/PNG export formats
  */
-const ExportModal = ({ graphSize, theme, onToggle, visible }) => {
-  if (!visible.exportBtn) {
+const ExportModal = ({ graphSize, theme, onToggle, visible, display }) => {
+  if (!display.exportBtn) {
     return null;
   }
   return (
@@ -46,6 +46,7 @@ const ExportModal = ({ graphSize, theme, onToggle, visible }) => {
 export const mapStateToProps = (state) => ({
   graphSize: state.graph.size || {},
   visible: state.visible,
+  display: state.display,
   theme: state.theme,
 });
 

src/components/export-modal/export-modal.test.js

@@ -29,10 +29,12 @@ describe('ExportModal', () => {
       }),
       theme: expect.stringMatching(/light|dark/),
       visible: expect.objectContaining({
-        exportBtn: expect.any(Boolean),
         exportModal: expect.any(Boolean),
         settingsModal: expect.any(Boolean),
       }),
+      display: expect.objectContaining({
+        exportBtn: expect.any(Boolean),
+      }),
     };
     expect(mapStateToProps(mockState.spaceflights)).toEqual(expectedResult);
   });

src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.js

@@ -32,6 +32,7 @@ export const FlowchartPrimaryToolbar = ({
   onToggleTextLabels,
   textLabels,
   visible,
+  display,
   visibleLayers,
   expandedPipelines,
   onToggleExpandAllPipelines,
@@ -59,7 +60,7 @@ export const FlowchartPrimaryToolbar = ({
           icon={LabelIcon}
           labelText={`${textLabels ? 'Hide' : 'Show'} text labels`}
           onClick={() => onToggleTextLabels(!textLabels)}
-          visible={visible.labelBtn}
+          visible={display.labelBtn}
         />
         <IconButton
           active={visibleLayers}
@@ -71,7 +72,7 @@ export const FlowchartPrimaryToolbar = ({
           icon={LayersIcon}
           labelText={`${visibleLayers ? 'Hide' : 'Show'} layers`}
           onClick={() => onToggleLayers(!visibleLayers)}
-          visible={visible.layerBtn}
+          visible={display.layerBtn}
         />
         <IconButton
           active={expandedPipelines}
@@ -88,7 +89,7 @@ export const FlowchartPrimaryToolbar = ({
           }
           data-test={'expand-all-pipelines-toggle'}
           onClick={handleToggleExpandAllPipelines}
-          visible={visible.expandPipelinesBtn}
+          visible={display.expandPipelinesBtn}
         />
         <IconButton
           ariaLabel="Export graph as SVG or PNG"
@@ -97,7 +98,7 @@ export const FlowchartPrimaryToolbar = ({
           icon={ExportIcon}
           labelText="Export visualisation"
           onClick={() => onToggleExportModal(true)}
-          visible={visible.exportBtn}
+          visible={display.exportBtn}
         />
       </PrimaryToolbar>
     </>
@@ -109,6 +110,7 @@ export const mapStateToProps = (state) => ({
   displaySidebar: state.display.sidebar,
   textLabels: state.textLabels,
   visible: state.visible,
+  display: state.display,
   visibleLayers: Boolean(getVisibleLayerIDs(state).length),
   expandedPipelines: state.expandAllPipelines,
 });

src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.test.js

@@ -18,25 +18,25 @@ describe('PrimaryToolbar', () => {
     expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(5);
   });
 
-  it('hides all buttons (except menu button) when visible prop is false for each of them', () => {
-    const visible = {
+  it('hides all buttons (except menu button) when display prop is false for each of them', () => {
+    const display = {
       labelBtn: false,
       layerBtn: false,
       exportBtn: false,
       expandPipelinesBtn: false,
     };
     const wrapper = setup.mount(<ConnectedFlowchartPrimaryToolbar />, {
-      visible,
+      display,
     });
     expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(1);
   });
 
-  it('hides one button when visible prop is false for one of them', () => {
-    const visible = {
+  it('hides one button when display prop is false for one of them', () => {
+    const display = {
       labelBtn: false,
     };
     const wrapper = setup.mount(<ConnectedFlowchartPrimaryToolbar />, {
-      visible,
+      display,
     });
     expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(4);
   });
@@ -57,6 +57,7 @@ describe('PrimaryToolbar', () => {
         displaySidebar: true,
         textLabels: mockState.spaceflights.textLabels,
         visible: mockState.spaceflights.visible,
+        display: mockState.spaceflights.display,
         [callback]: mockFn,
       };
       const wrapper = setup.mount(<FlowchartPrimaryToolbar {...props} />);
@@ -73,14 +74,16 @@ describe('PrimaryToolbar', () => {
       expandedPipelines: expect.any(Boolean),
       displaySidebar: true,
       visible: expect.objectContaining({
-        exportBtn: expect.any(Boolean),
         exportModal: expect.any(Boolean),
         metadataModal: expect.any(Boolean),
         settingsModal: expect.any(Boolean),
+        sidebar: expect.any(Boolean),
+      }),
+      display: expect.objectContaining({
+        exportBtn: expect.any(Boolean),
         labelBtn: expect.any(Boolean),
         layerBtn: expect.any(Boolean),
         expandPipelinesBtn: expect.any(Boolean),
-        sidebar: expect.any(Boolean),
       }),
       visibleLayers: expect.any(Boolean),
     };

src/components/global-toolbar/global-toolbar.test.js

@@ -47,16 +47,11 @@ describe('GlobalToolbar', () => {
       theme: expect.stringMatching(/light|dark/),
       visible: {
         code: false,
-        exportBtn: true,
         exportModal: false,
         graph: true,
-        labelBtn: true,
-        layerBtn: true,
         miniMap: true,
-        miniMapBtn: true,
         modularPipelineFocusMode: null,
         metadataModal: false,
-        expandPipelinesBtn: true,
         settingsModal: false,
         shareableUrlModal: false,
         sidebar: true,

src/components/minimap-toolbar/minimap-toolbar.js

@@ -34,7 +34,7 @@ export const MiniMapToolbar = ({
             id="minimap-toggle-icon"
             labelText={`${visible.miniMap ? 'Hide' : 'Show'} minimap`}
             onClick={() => onToggleMiniMap(!visible.miniMap)}
-            visible={visible.miniMapBtn}
+            visible={displayMiniMap}
           />
         )}
         <IconButton

src/components/minimap-toolbar/minimap-toolbar.test.js

@@ -52,7 +52,6 @@ describe('MiniMapToolbar', () => {
       chartZoom: expect.any(Object),
       visible: expect.objectContaining({
         miniMap: expect.any(Boolean),
-        miniMapBtn: expect.any(Boolean),
       }),
     };
     expect(mapStateToProps(mockState.spaceflights)).toEqual(expectedResult);

src/components/minimap/minimap.js

@@ -329,7 +329,9 @@ export class MiniMap extends Component {
     return (
       <div
         className="pipeline-minimap-container"
-        style={this.props.visible ? transformStyle : {}}
+        style={
+          this.props.visible && this.props.displayMiniMap ? transformStyle : {}
+        }
       >
         <div
           className="pipeline-minimap kedro"
@@ -402,6 +404,7 @@ const emptyGraphSize = {};
 
 export const mapStateToProps = (state, ownProps) => ({
   visible: state.visible.miniMap,
+  displayMiniMap: state.display.miniMap,
   mapSize: getMapSize(state),
   clickedNode: state.node.clicked,
   chartSize: getChartSize(state),

src/components/minimap/minimap.test.js

@@ -145,6 +145,7 @@ describe('MiniMap', () => {
   it('maps state to props', () => {
     const expectedResult = {
       visible: expect.any(Boolean),
+      displayMiniMap: expect.any(Boolean),
       mapSize: expect.any(Object),
       clickedNode: null,
       chartSize: expect.any(Object),

src/components/settings-modal/settings-modal.test.js

@@ -42,7 +42,6 @@ describe('SettingsModal', () => {
   it('maps state to props', () => {
     const expectedResult = {
       visible: expect.objectContaining({
-        exportBtn: expect.any(Boolean),
         exportModal: expect.any(Boolean),
         settingsModal: expect.any(Boolean),
       }),

src/store/initial-state.js

@@ -32,16 +32,11 @@ export const createInitialState = () => ({
   },
   visible: {
     code: false,
-    exportBtn: true,
     exportModal: false,
     graph: true,
-    labelBtn: true,
-    layerBtn: true,
     metadataModal: false,
     miniMap: true,
-    miniMapBtn: true,
     modularPipelineFocusMode: null,
-    expandPipelinesBtn: true,
     settingsModal: false,
     shareableUrlModal: false,
     sidebar: window.innerWidth > sidebarWidth.breakpoint,
@@ -50,7 +45,10 @@ export const createInitialState = () => ({
     globalToolbar: true,
     sidebar: true,
     miniMap: true,
-    expandAllPipelines: false,
+    expandPipelinesBtn: true,
+    exportBtn: true,
+    labelBtn: true,
+    layerBtn: true,
   },
   zoom: {},
   runsMetadata: {},
@@ -251,9 +249,7 @@ const getInitialState = (props = {}) => {
   const urlParams = parseUrlParameters();
   const nonPipelineState = prepareNonPipelineState(props, urlParams);
 
-  const expandAllPipelines =
-    nonPipelineState.display.expandAllPipelines ||
-    nonPipelineState.expandAllPipelines;
+  const expandAllPipelines = nonPipelineState.expandAllPipelines;
 
   const pipelineState = preparePipelineState(
     props.data,

src/store/initial-state.test.js

@@ -132,7 +132,7 @@ describe('getInitialState', () => {
       textLabels: true,
       theme: 'dark',
       expandAllPipelines: false,
-      visible: {
+      display: {
         exportBtn: true,
         labelBtn: true,
         layerBtn: true,
@@ -149,7 +149,7 @@ describe('getInitialState', () => {
       })
     ).toMatchObject({
       theme: 'light',
-      visible: { labelBtn: true },
+      display: { labelBtn: true },
     });
   });
 

tools/test-lib/react-app/VizComponent.js

@@ -6,8 +6,8 @@ function VizComponent({ data }) {
     <div style={{ height: `90vh`, width: `100%` }}>
       <KedroViz
         data={data}
+        expandAllPipelines={false}
         display={{
-          expandAllPipelines: false,
           globalToolbar: true,
           miniMap: false,
           sidebar: true,