Exposing internal state via Kedro Viz React component props

[jitu5]

Resolves #1745

Description

This PR aims to enhance the flexibility and functionality of the Kedro Viz React component by exposing internal Redux state to component users through props.
Existing props structure:

<KedroViz
    data={json}
    display={
      globalToolbar: false,
      miniMap: false,
      expandAllPipelines: false,
      sidebar: false
    }}
    visible={
      labelBtn: false,
      layerBtn: false,
      exportBtn: false,
      pipelineBtn: false,
      sidebar: false,
    }}
    theme="dark"
 />

New props structure:

<KedroViz
    data={json}
    options={
      display: {
        globalToolbar: true,
        sidebar: true,
        miniMap: true,
        expandAllPipelines: false,
      },
      visible={
        labelBtn: false,
        layerBtn: false,
        exportBtn: false,
        pipelineBtn: false,
        sidebar: false,
      },
      theme: "dark",
      tag: {
        enabled: {companies: true}
      },
      nodeType: {
        disabled: {parameters: true}
      }
    }
 />

The new structure consolidates display, visible and other settings under a single options object, making the component's API more organized and scalable.

Note: Other Redux state structure changes like moving some state from visible to display is done under PR #1983.

Development notes

• Generic action UPDATE_STATE_FROM_OPTIONS created to update redux state coming from react props.
• Except data all existing props will wrap under a options
• options.display.sidebar will remove the sidebar & actionToolbar
• nonPipelineState and pipelineState will not accept props
• props.options will merge to existing state in getInitialState

kedro viz UI annotation:

QA notes

You can test new options prop in src/components/container.js as <App /> is the entry point or top level component for a standalone use case.

• I created 3 state tags, nodeTypes and theme to test when it changes on setTimeout after 10 secs.
• Pass options prop to component to test all possible cases.

import React, { useEffect } from 'react';
import App from '../app';
import getPipelineData from '../../utils/data-source';
import './container.scss';

/**
 * Top-level component for the use-case where Kedro-Viz is run as a standalone
 * app rather than imported as a library/package into a larger application.
 */
const Container = () => {
  const [tags, setTags] = React.useState({
    enabled: { companies: true, shuttles: true },
  });
  const [nodeTypes, setNodeTypes] = React.useState({
    disabled: {
      parameters: false,
      task: false,
      data: false,
    },
  });
  const [theme, setTheme] = React.useState('dark');

  useEffect(() => {
    setTimeout(() => {
      setTags({
        enabled: { evaluate: true, companies: false },
      });
      setNodeTypes({
        disabled: {
          parameters: true,
          task: true,
          data: false,
        },
      });
      setTheme('light');
    }, 10000);
  }, [theme, tags, nodeTypes]);

  return (
    <>
      <App
        data={getPipelineData()}
        options={{
          display: {
            globalToolbar: false,
            miniMap: true,
            expandAllPipelines: true,
            sidebar: true,
          },
          tag: tags,
          nodeType: nodeTypes,
          theme: theme,
        }}
      />
    </>
  );
};

export default Container;

Checklist

• Read the contributing guidelines
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added new entries to the RELEASE.md file
• Added tests to cover my changes

[ravi-kumar-pilla]

Hi @jitu5 ,

Great to see the component having more customizable options. Some suggestions -

1. For me, the props on the component seem to be confusing with the actual React props. May be we rename it
2. Since the customizable options seem to be multi-level nesting objects (like - nodeType: {
   disabled: {parameters: true}
   }), I would prefer having the customizable options separately on the component level rather than nesting inside an object like props

Thank you

[jitu5]

Hi @jitu5 ,

Great to see the component having more customizable options. Some suggestions -

1. For me, the props on the component seem to be confusing with the actual React props. May be we rename it
2. Since the customizable options seem to be multi-level nesting objects (like - nodeType: {
   disabled: {parameters: true}
   }), I would prefer having the customizable options separately on the component level rather than nesting inside an object like props

Thank you

1. I was also thinking to rename props to something else like config or options wdyt @ravi-kumar-pilla @rashidakanchwala

2. You mean on the component level like this?

<KedroViz
    data={json}
    tag: {
      enabled: {companies: true}
    },
    nodeType: {
      disabled: {parameters: true}
    }
    props={
      theme: "dark",
      display: {
        ...
      }
    }
 />

Initially, we considered exposing specific props like nodeType or tag and handling them internally. However, we decided against this approach because we wanted to simplify the merging process with the internal Redux state. By passing a single props object that mirrors the structure of Kedro Viz's internal Redux state, we can easily merge it directly into the internal Redux state. This approach reduces complexity and keeps the component interface clean.

[ravi-kumar-pilla]

1. I was also thinking to rename props to something else like config or options wdyt @ravi-kumar-pilla @rashidakanchwala

I am fine with having any of the above mentioned names

2. You mean on the component level like this?

<KedroViz
    data={json}
    tag: {
      enabled: {companies: true}
    },
    nodeType: {
      disabled: {parameters: true}
    }
    props={
      theme: "dark",
      display: {
        ...
      }
    }
 />

My main concern is when a user wants to define a prop, he/she needs to do the nesting properly in defining. Seems like this way of nesting is cleaner to merge with what we have now

[rashidakanchwala]

1. I was also thinking to rename props to something else like config or options wdyt @ravi-kumar-pilla @rashidakanchwala

I am fine with having any of the above mentioned names

2. You mean on the component level like this?

<KedroViz
    data={json}
    tag: {
      enabled: {companies: true}
    },
    nodeType: {
      disabled: {parameters: true}
    }
    props={
      theme: "dark",
      display: {
        ...
      }
    }
 />

My main concern is when a user wants to define a prop, he/she needs to do the nesting properly in defining. The other issue which I thought was while checking change in state, if the nested json changes, we should be checking for the change recursively (I mean checking the nested value) right ? Thank you

Yes, that's true. If we don't nest the configurations, we will need to handle and merge each config individually in our code. By exposing our configurations in a nested dict instead of at the top level, users must follow the exact config structure to override the default settings. This minimizes our code, as we can merge the dict as a whole rather than merging each config individually. I know this may not be the most user-friendly, but there are very few advanced JavaScript users who need this feature, so this approach allows us to balance exposing our config whilst minimizing unnecessary code.

i am ok calling it options

[jitu5]

My main concern is when a user wants to define a prop, he/she needs to do the nesting properly in defining. Seems like this way of nesting is cleaner to merge with what we have now

Yes, for the nested props we have docs for the right structure and it will help the user.

[ravi-kumar-pilla]

Hi @jitu5 , I have commented minor changes and few questions for my understanding. It would be helpful, if you can add QA notes on how to test some changes in this PR (only basic smoke test and not an exhaustive testing). Thank you

[ravi-kumar-pilla]

Hi @jitu5 , Thanks for including the QA steps, it is really helpful. I observed few things -

1. When {display: {sidebar:false}}, it does not even show the minimap toolbar. Is this expected ? I was thinking it will only hide the menu and search area

2. Lets say I have the below options -

options={{
          display: {
            globalToolbar: true,
            miniMap: false,
            expandAllPipelines: false,
            sidebar: true,
          },
          visible: {
            pipelineBtn: true,
            exportBtn: true,
            labelBtn: true,
            layerBtn: true,
            sidebar: false,
            miniMap: true,
          },
}}

The miniMap display is set to false but the miniMap on visible is set to true, this hides the miniMap icon but continues to show the map as below (this seems like a bug) -

3. With the above options, expandAllPipelines: false/true, always shows the icon (this seems like a bug)
4. I have a question on what does the sidebar means ? Is it the entire block on the left excluding the global toolbar ? Or is it just the search + menu + filters section ? Does sidebar include miniMap area as the miniMap area is also hidden when sidebar display is false.
5. From the QA test after the timeout of 10sec, the UI is getting updated but the stateful URL still shows the initial load URL. For example,

setNodeTypes({
        disabled: {
          parameters: true,
          task: true,
          data: false,
        },
      });

The above change affects the UI but the URL remains as - http://localhost:4141/?types=parameters,nodes,datasets

setTags({
        enabled: { evaluate: true, companies: false },
      });

Same is the case for tags - http://localhost:4141/?types=parameters,nodes,datasets&pid=__default__&tags=companies,evaluate,shuttles&expandAllPipelines=false

Thank you

[Huongg]

all looks good to me, thanks @jitu5. Just 2 small questions about using loadash

[jitu5]

Hi @jitu5 , Thanks for including the QA steps, it is really helpful. I observed few things -

1. When {display: {sidebar:false}}, it does not even show the minimap toolbar. Is this expected ? I was thinking it will only hide the menu and search area

2. Lets say I have the below options -

options={{
          display: {
            globalToolbar: true,
            miniMap: false,
            expandAllPipelines: false,
            sidebar: true,
          },
          visible: {
            pipelineBtn: true,
            exportBtn: true,
            labelBtn: true,
            layerBtn: true,
            sidebar: false,
            miniMap: true,
          },
}}

The miniMap display is set to false but the miniMap on visible is set to true, this hides the miniMap icon but continues to show the map as below (this seems like a bug) -

3. With the above options, expandAllPipelines: false/true, always shows the icon (this seems like a bug)
4. I have a question on what does the sidebar means ? Is it the entire block on the left excluding the global toolbar ? Or is it just the search + menu + filters section ? Does sidebar include miniMap area as the miniMap area is also hidden when sidebar display is false.
5. From the QA test after the timeout of 10sec, the UI is getting updated but the stateful URL still shows the initial load URL. For example,

setNodeTypes({
        disabled: {
          parameters: true,
          task: true,
          data: false,
        },
      });

The above change affects the UI but the URL remains as - http://localhost:4141/?types=parameters,nodes,datasets

setTags({
        enabled: { evaluate: true, companies: false },
      });

Same is the case for tags - http://localhost:4141/?types=parameters,nodes,datasets&pid=__default__&tags=companies,evaluate,shuttles&expandAllPipelines=false

Thank you

@ravi-kumar-pilla

Please refer the annotation image

1. As miniMap toolbar component is part of the <Sidebar> parent component. In flowchart-wrapper.js, we are dong below, if {display: {sidebar:false}}, <Sidebar> will not be render hence miniMap toolbar component will also be removed.

2. We are not documenting {visible: {miniMap:<Bool>}} hence not exposing it to user for now, let me explain how each prop related to miniMap
   a. {display: {miniMap:<Bool>}} => hide/show miniMap button on primary toolbar (added in React doc PR.)
   b. {visible: {miniMap:<Bool>}} => hide/show miniMap toolbar
   c. {visible: {miniMapBtn:<Bool>}} => hide/show miniMap control buttons on primary toolbar like (miniMap button, zoom out, zoom in and zoom reset)

As mentioned above, we are now only documenting {display: {miniMap: <Bool>}}, which shows or hides the miniMap button. If we need to change any behavior, such as specifying which prop does what and vice versa related to miniMap, we will need to refactor. As discussed with Rashida, For exposing state PR we wanted to do it with the minimum changes. @rashidakanchwala please correct me if I am wrong.

3. here {display: {expandAllPipelines: <Bool>}} does not hide or show expandAllPipelines icon instead Expand/Collapse modular pipelines on first load, added in React doc PR.

4. Please refer Answer 1 and annotation image added.

5. Since this is in embedded mode, updating the URL does not significantly affect the user of the React component. However, I can update the URL if the team thinks otherwise.

[ravi-kumar-pilla]

Looks good to me. Thank you for all the refactor here !! I would like to test the combined branch as individual branches seem to miss few updates.

[Huongg]

LGTM too, thanks @jitu5

[jitu5]

Closing this PR as it already merged with #1992 to main.