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 in text for users to reboot their Kedro-Viz after using the expandAllPipelines flag #792

Closed
1 task done
studioswong opened this issue Mar 23, 2022 · 4 comments
Closed
1 task done
Assignees
Labels
Issue: Bug Report Javascript Pull requests that update Javascript code

Comments

@studioswong
Copy link
Contributor

Description

The current settings panel does not have a 'Apply' button where settings are applied immediately on toggling the settings. However, the setup of some feature flags is designed to be implement on load, such as the sizeWarning and the newly introduced expandAllPipelines flags, which causes general confusion as toggling the settings on the settings panel would not cause the settings to be immediately reflected until the next refresh of the page.

Generally it is confusing for the user to confirm on whether the settings have been applied without the 'Apply and Close' button.

This is to report the need to implement a 'Apply' button for the panel to solve the need to reload the app on updating feature flag settings ( the button is present in the design for the settings panel here, QB access only)

Context

Without the button, updating the feature flag settings would not allow the relevant features to be reflected until the app reloads.

Expected Result

A 'Apply and Close' button should be present, where on pressing the button it allows the app to reload and apply the behaviour introduced by the feature flags.

Actual Result

Currently, toggling the feature flag would not cause immediate effect until the user refreshs the app.

Checklist

  • Include labels so that we can categorise your issue
@studioswong studioswong added Issue: Bug Report Type: Fix Javascript Pull requests that update Javascript code labels Mar 23, 2022
@studioswong studioswong changed the title Add a 'Apply and Close' button for the settings panel to allow feature flags settings to be reflected in the app Add a 'Apply and Close' button for the settings panel to allow feature flags settings to be reflected Mar 23, 2022
@tynandebold
Copy link
Member

tynandebold commented Apr 6, 2022

In the settings panel, the toggle for Pretty Name does actually change the state of the app when toggling between off and on. I wonder if we can do the same thing for the new expandAllPipelines flag?

@studioswong
Copy link
Contributor Author

studioswong commented Apr 11, 2022

Unfortunately this cannot be applied to the expandAllPipelines flag as the setup for expanding all pipelines is applied during state initialisation and not through actions ( which the toggle for Pretty Name is done through actions) - expanding all pipelines in app will be a different feature on its own 😄 ( this is also the same case for the large pipeline warning as well).

There is also another quick solution, which is to add the wording (applies on refresh of app) in the description of both experiment features.

That said, I do agree with design that having a 'Apply Changes' button for the settings panel provides more clarity and assurance of applying the settings, especially given the panel is a modal - I will have a quick chat with design and we can decide on a the best solution.

@studioswong
Copy link
Contributor Author

update: as per discussion in our team sprint planning, we will go for the quick win solution of modifying the description, and later on to implement the expandAllPipelines as a in-app state to allow the users to toggle the expansion of all pipelines within the app ( not just only on first load.) - that will be filed under a separate issue.

@yetudada yetudada changed the title Add a 'Apply and Close' button for the settings panel to allow feature flags settings to be reflected Add in text for users to reboot their Kedro-Viz after using the expandAllPipelines flag Apr 12, 2022
@tynandebold
Copy link
Member

Closed with completion of #875.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Bug Report Javascript Pull requests that update Javascript code
Projects
None yet
Development

No branches or pull requests

2 participants