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

Add disable preview toggle in settings panel #1977

Merged
merged 38 commits into from
Jul 25, 2024
Merged

Conversation

SajidAlamQB
Copy link
Contributor

@SajidAlamQB SajidAlamQB commented Jul 10, 2024

Description

Related to: #1921

This feature enables users to control data previews for all nodes from the settings panel of Kedro-Viz when running kedro viz locally.

After running Kedro-Viz locally, dataset previews for all nodes will be enabled by default. However, users will have the ability to enable or disable this feature from the Kedro-Viz settings panel.

Development Notes

Backend Changes:

  • Introduced a new API endpoint /api/preferences to handle user preferences updates.
  • Created a UserPreference model to define the preference schema.
  • Implemented a new function in router.py to update the showDatasetPreviews preference and set the class variable accordingly.
  • Adjusted the logic in responses.py to use the new preference for determining dataset preview settings.

Frontend Changes:

  • Added a new showDatasetPreviews toggle in the Redux state to manage the setting.
  • Updated the SettingsModal component to include the showDatasetPreviews toggle.

How to Test:

  • Run kedro viz locally via
  • Open the Kedro-Viz settings panel.
  • Toggle the Dataset Previews setting on and off.
  • Verify that the dataset previews for all nodes are enabled/disabled based on the setting.
  • Check the network tab to confirm that Preferences API is called and that the preferences are correctly updated on the backend. (Might need to toggle preserve logs to view it as saving in the settings menu refreshes the page and clears network logs)

New dataset preview toggle:

image

New API endpoint:

image

QA notes

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

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
@SajidAlamQB SajidAlamQB self-assigned this Jul 10, 2024
SajidAlamQB and others added 5 commits July 10, 2024 08:31
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
@SajidAlamQB SajidAlamQB marked this pull request as ready for review July 15, 2024 13:51
@Huongg
Copy link
Contributor

Huongg commented Jul 16, 2024

hey @SajidAlamQB what's the best way to test your branch locally? I run kedro viz run in demo-project but I guess it won't reflect any changes from the FE

@SajidAlamQB
Copy link
Contributor Author

hey @SajidAlamQB what's the best way to test your branch locally? I run kedro viz run in demo-project but I guess it won't reflect any changes from the FE

Hi, I'm making some changes at the moment, but I've been using make build and make run to test the changes locally.

@Huongg
Copy link
Contributor

Huongg commented Jul 16, 2024

hey @SajidAlamQB what's the best way to test your branch locally? I run kedro viz run in demo-project but I guess it won't reflect any changes from the FE

Hi, I'm making some changes at the moment, but I've been using make build and make run to test the changes locally.

oh i see, maybe i got it wrong but from the description it says when you run "kedro viz run". So i expect if i run "kedro viz run" the behaviour will be different from if i run "kedro viz build" etc. Is it the case?

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
@SajidAlamQB
Copy link
Contributor Author

oh i see, maybe i got it wrong but from the description it says when you run "kedro viz run". So i expect if i run "kedro viz run" the behaviour will be different from if i run "kedro viz build" etc. Is it the case?

It should be the same behaviour, I've made quite a few changes and updated the description might be worth testing again.

@SajidAlamQB SajidAlamQB requested a review from Huongg July 17, 2024 09:40
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
@stephkaiser
Copy link

Thanks Sajid! some very minor design QA comments:

  • could we make it so that 'Previews" has a lowercase p - so "Dataset previews"
  • and can we change the description to "Display preview data for all datasets."

SajidAlamQB and others added 2 commits July 18, 2024 09:42
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
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.

thank you @SajidAlamQB. I've left a few comments. Let me know if you have any questions about them

src/reducers/index.js Outdated Show resolved Hide resolved
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
SajidAlamQB and others added 3 commits July 24, 2024 03:14
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Copy link
Contributor

@jitu5 jitu5 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @SajidAlamQB

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
SajidAlamQB and others added 3 commits July 24, 2024 15:11
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
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.

Thank you for addressing all the PR comments @SajidAlamQB . LGTM and works well 💯

@SajidAlamQB SajidAlamQB merged commit f75f16a into main Jul 25, 2024
25 checks passed
@SajidAlamQB SajidAlamQB deleted the feat/disable-preview branch July 25, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants