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.

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

For Test and Review: React component work all merged in single PR. #1992

Merged
merged 88 commits into from
Jul 22, 2024

Conversation

jitu5
Copy link
Contributor

@jitu5 jitu5 commented Jul 19, 2024

Description

This is draft PR created just for test and review to see all the changes related to React component work together in one PR.

Exposing internal state: #1969
metadataPanel prop: #1965
MiniMapBtn prop: #1982
miniMap & pipelineBtn prop: #1983
Documentation: #1954

All the above branches are merged into this and once all the above is approved, I will merge this into main.

Development notes

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, expAll]);

  return (
    <>
      <App
        data={getPipelineData()}
        options={{
          expandAllPipelines: expAll,
          display: {
            globalNavigation: true,
            sidebar: true,
            miniMap: true,
            expandPipelinesBtn: true,
            exportBtn: true,
            labelBtn: true,
            layerBtn: true,
            zoomToolbar: true,
            metadataPanel: 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

jitu5 and others added 30 commits June 21, 2024 20:09
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Co-authored-by: rashidakanchwala <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Co-authored-by: Tynan DeBold <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Co-authored-by: Tynan DeBold <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Co-authored-by: rashidakanchwala <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
# Conflicts:
#	src/components/app/app.js
#	src/components/flowchart-wrapper/flowchart-wrapper.js
Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
@jitu5 jitu5 self-assigned this Jul 19, 2024
@jitu5 jitu5 added the Javascript Pull requests that update Javascript code label Jul 19, 2024
@ravi-kumar-pilla ravi-kumar-pilla self-requested a review July 20, 2024 22:55
Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

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

Apart from the [Nit] comment here, everything works as expected in local. Great work @jitu5 💯 . Thank you

Copy link
Contributor

@Huongg Huongg left a comment

Choose a reason for hiding this comment

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

hey i tested it locally from my side and everything is working fine. Thank you @jitu5

@jitu5 jitu5 marked this pull request as ready for review July 22, 2024 12:54
Signed-off-by: Jitendra Gundaniya <[email protected]>
@jitu5 jitu5 merged commit 6534297 into main Jul 22, 2024
25 checks passed
@jitu5 jitu5 deleted the all-react-branchs branch July 22, 2024 13:16
@noklam noklam changed the title [DRAFT]For Test and Review: React component work all merged in single PR. For Test and Review: React component work all merged in single PR. Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants