-
Notifications
You must be signed in to change notification settings - Fork 113
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
Feature/slicing pipeline add notification #2003
Feature/slicing pipeline add notification #2003
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @SajidAlamQB, The code is looking pretty good! 🔥
I did notice one thing: the notification transition is a bit glitchy. In the video, it looks like it gets stuck and doesn't move to the left smoothly in one transition, but it seems fine when you shift-hold and click. 🤔 I wonder if the translateX isn't being applied immediately or if the transformX calculated number is changing too frequently.
notification.transform.mov
src/components/flowchart/components/sliced-pipeline-action-bar/sliced-pipeline-action-bar.js
Outdated
Show resolved
Hide resolved
src/components/flowchart/components/sliced-pipeline-action-bar/sliced-pipeline-action-bar.js
Outdated
Show resolved
Hide resolved
src/components/flowchart/components/sliced-pipeline-action-bar/sliced-pipeline-action-bar.js
Outdated
Show resolved
Hide resolved
I am not able to see this on my end, the transition seems smooth when we click away and when we shift + click. |
hey I've updated the transition timing from 0.7s to 0.2s and it looks a lot smoother from my side now. I think 0.7s was to slow so it's showing all the different movements, that's why i can see it's a bit glitchy from myside |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @SajidAlamQB thanks for making all the changes, I've left some comments on how you can simplify the code and it will also help to fix the bug that we discussed earlier on the call. Let me know if you'd like to discuss them further on the call 😄
...omponents/flowchart/components/sliced-pipeline-action-bar/sliced-pipeline-action-bar.test.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
@SajidAlamQB @Huongg |
good question, can we test it together tomorrow or sometimes this week? If it's working fine and this functionality is a part of flowchart, I don't see why we can't have this? Do you see any risks we will take if we don't disable it @jitu5 ? |
@Huongg In most of the case When Kedro viz is used in embedded mode the main purpose is readonly or only flowchart visualisation, But I am not fully sure should we disabled it or not. @rashidakanchwala wdyt ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @SajidAlamQB @Huongg
It's a bit tricky to determine if it's suitable for embedded mode without actually seeing it in action. In my mind, it seems like there wouldn't be any harm in including it in the standalone flowchart, since that's where most of the interaction happens. However, I can also understand the reasons for disabling it, especially if we decide to turn off the metadata panel. |
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Hi @SajidAlamQB , As I was testing the fix for - #2003 (comment)
|
Signed-off-by: Sajid Alam <[email protected]>
Should be fixed now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !! Thank you @SajidAlamQB
…ication Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
* set actions - filters Signed-off-by: Huong Nguyen <huongg1409@gmail> * set reducers - filters Signed-off-by: Huong Nguyen <huongg1409@gmail> * set reducers - filtered-pipeline Signed-off-by: Huong Nguyen <huongg1409@gmail> * user getFilteredPipeline in disabled Signed-off-by: Huong Nguyen <huongg1409@gmail> * set eslint Signed-off-by: Huong Nguyen <huongg1409@gmail> * first setup in flowchart Signed-off-by: Huong Nguyen <huongg1409@gmail> * include isDisabledViaFilters in disabled Signed-off-by: Huong Nguyen <huongg1409@gmail> * highlight multi-select nodes Signed-off-by: Huong Nguyen <huongg1409@gmail> * use local state instead Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove extra action Signed-off-by: Huong Nguyen <huongg1409@gmail> * shifting the logic around for shift hold the node Signed-off-by: Huong Nguyen <huongg1409@gmail> * create a separate state for highlighting node Signed-off-by: Huong Nguyen <huongg1409@gmail> * firt working version of highlighting the node list Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove the extra selector and action Signed-off-by: Huong Nguyen <huongg1409@gmail> * first draft of highlighting the node list Signed-off-by: Huong Nguyen <huongg1409@gmail> * hide the slicing button Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove extra node selector Signed-off-by: Huong Nguyen <huongg1409@gmail> * set click outside to clear highlighting node Signed-off-by: Huong Nguyen <huongg1409@gmail> * slice pipeline component Signed-off-by: Huong Nguyen <huongg1409@gmail> * handle click on the third node Signed-off-by: Huong Nguyen <huongg1409@gmail> * reset filter nodes on click on the node list Signed-off-by: Huong Nguyen <huongg1409@gmail> * create utils function for filtering nodes in draw.js Signed-off-by: Huong Nguyen <huongg1409@gmail> * adding comments for componentDidUpdate Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove un-used selectors Signed-off-by: Huong Nguyen <huongg1409@gmail> * rename filteredPipeline state and classname Signed-off-by: Huong Nguyen <huongg1409@gmail> * include filters in normalize data Signed-off-by: Huong Nguyen <huongg1409@gmail> * update normalize data Signed-off-by: Huong Nguyen <huongg1409@gmail> * test for reducers Signed-off-by: Huong Nguyen <huongg1409@gmail> * write test for selector Signed-off-by: Huong Nguyen <huongg1409@gmail> * changes to test the tests locally Signed-off-by: Huong Nguyen <huongg1409@gmail> * responsive cta button Signed-off-by: Huong Nguyen <huongg1409@gmail> * update failed test Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix failed e2e tests Signed-off-by: Huong Nguyen <huongg1409@gmail> * adding transformX value for button Signed-off-by: Huong Nguyen <huongg1409@gmail> * add slice btn functionality Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove unused props Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix console error where the same id applies twice Signed-off-by: Huong Nguyen <huongg1409@gmail> * prevent resetting on onclick slice btn Signed-off-by: Huong Nguyen <huongg1409@gmail> * first working version of dilerting node list Signed-off-by: Huong Nguyen <huongg1409@gmail> * filtering node list on the left Signed-off-by: Huong Nguyen <huongg1409@gmail> * filter out modular pipeline if no nodes selected Signed-off-by: Huong Nguyen <huongg1409@gmail> * include reset button Signed-off-by: Huong Nguyen <huongg1409@gmail> * cy:test close the hint feature so can click on a node Signed-off-by: Huong Nguyen <huongg1409@gmail> * run command selector Signed-off-by: Huong Nguyen <huongg1409@gmail> * include command copier Signed-off-by: Huong Nguyen <huongg1409@gmail> * UI for command line Signed-off-by: Huong Nguyen <huongg1409@gmail> * avoid reseting when clicking on a node while filters is activated Signed-off-by: Huong Nguyen <huongg1409@gmail> * Add comment Signed-off-by: Huong Nguyen <huongg1409@gmail> * test to cover the run-command selector Signed-off-by: Huong Nguyen <huongg1409@gmail> * no holdshift click on nodes when filters are applied Signed-off-by: Huong Nguyen <huongg1409@gmail> * rename component and apply light theme Signed-off-by: Huong Nguyen <huongg1409@gmail> * move theme to shared variable Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix typo Signed-off-by: Huong Nguyen <huongg1409@gmail> * close the hint feature in cy test Signed-off-by: Huong Nguyen <huongg1409@gmail> * lower case for one of the cy test Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove pretify name as its already set Signed-off-by: Huong Nguyen <huongg1409@gmail> * update component name Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix merge conflict and update classname for FilteredPipelineActionBar Signed-off-by: Huong Nguyen <huongg1409@gmail> * update classnames to match the new component name Signed-off-by: Huong Nguyen <huongg1409@gmail> * change the styling for filtered nodes Signed-off-by: Huong Nguyen <huongg1409@gmail> * refactor the logic to make it clearer Signed-off-by: Huong Nguyen <huongg1409@gmail> * temporary solution for styling highlighting state Signed-off-by: Huong Nguyen <huongg1409@gmail> * update name for selector and flowchart, draw and node-list components Signed-off-by: Huong Nguyen <huongg1409@gmail> * rename for state and action Signed-off-by: Huong Nguyen <huongg1409@gmail> * update name for action bar Signed-off-by: Huong Nguyen <huongg1409@gmail> * update name for css Signed-off-by: Huong Nguyen <huongg1409@gmail> * add notification Signed-off-by: Sajid Alam <[email protected]> * rename Signed-off-by: Sajid Alam <[email protected]> * Use SlicedPipelineActionBar Signed-off-by: Sajid Alam <[email protected]> * fix other merge conflict Signed-off-by: Huong Nguyen <huongg1409@gmail> * Update flowchart.test.js Signed-off-by: Sajid Alam <[email protected]> * styling for long run command Signed-off-by: Huong Nguyen <huongg1409@gmail> * adjust some styling Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove the styling for node list Signed-off-by: Huong Nguyen <huongg1409@gmail> * update failed test and use visibleSidebar instead Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix typo Signed-off-by: Huong Nguyen <huongg1409@gmail> * tidy up code Signed-off-by: Huong Nguyen <huongg1409@gmail> * update name in test Signed-off-by: Huong Nguyen <huongg1409@gmail> * add tests for util functions Signed-off-by: Huong Nguyen <huongg1409@gmail> * update logic for selector nodes Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove --over-write class on row as it's causing a bug Signed-off-by: Huong Nguyen <huongg1409@gmail> * merge Signed-off-by: Sajid Alam <[email protected]> * Update flowchart.js Signed-off-by: Sajid Alam <[email protected]> * remove overwrite styling as it causing a bug Signed-off-by: Huong Nguyen <huongg1409@gmail> * Revert "Merge branch 'feature/slicing-pipeline--run-command' into feature/slicing-notification" This reverts commit dd0d30b, reversing changes made to 5e02618. * include 2 new props in flowchart test Signed-off-by: Huong Nguyen <huongg1409@gmail> * change the position when meta panel opens Signed-off-by: Huong Nguyen <huongg1409@gmail> * update some colour with the new design changes Signed-off-by: Huong Nguyen <huongg1409@gmail> * selecting a node from bottom to top, the order should not matter Signed-off-by: Sajid Alam <[email protected]> * Update flowchart.js Signed-off-by: Sajid Alam <[email protected]> * center the notification Signed-off-by: Huong Nguyen <huongg1409@gmail> * center the action bar Signed-off-by: Huong Nguyen <huongg1409@gmail> * using existing function to update state Signed-off-by: Huong Nguyen <huongg1409@gmail> * use redux instead of local state Signed-off-by: Huong Nguyen <huongg1409@gmail> * tidy up code Signed-off-by: Sajid Alam <[email protected]> * update colours Signed-off-by: Huong Nguyen <huongg1409@gmail> * only style from to pipeline if both of them are defined Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix the fading styling Signed-off-by: Huong Nguyen <huongg1409@gmail> * add test Signed-off-by: Sajid Alam <[email protected]> * simplify the code Signed-off-by: Huong Nguyen <huongg1409@gmail> * changes based on review Signed-off-by: Sajid Alam <[email protected]> * update variable names Signed-off-by: Huong Nguyen <huongg1409@gmail> * adding comments to explain Signed-off-by: Huong Nguyen <huongg1409@gmail> * rename ActionBar to slicePipelineActionBar Signed-off-by: Sajid Alam <[email protected]> * reorder code Signed-off-by: Huong Nguyen <huongg1409@gmail> * update comment and function name Signed-off-by: Huong Nguyen <huongg1409@gmail> * set minimum transformX value Signed-off-by: Huong Nguyen <huongg1409@gmail> * update function name in test Signed-off-by: Huong Nguyen <huongg1409@gmail> * Adjust the transform value to 0.3s Signed-off-by: Huong Nguyen <huongg1409@gmail> * update config value for calculating postion Signed-off-by: Huong Nguyen <huongg1409@gmail> * update positioning logic Signed-off-by: Huong Nguyen <huongg1409@gmail> * removed unncesscary this.props.slicedPipeline Signed-off-by: Huong Nguyen <huongg1409@gmail> * include extra check for data.name Signed-off-by: Huong Nguyen <huongg1409@gmail> * add || rather than && Signed-off-by: Huong Nguyen <huongg1409@gmail> * add hover style for the slice button Signed-off-by: Huong Nguyen <huongg1409@gmail> * hover style for reset button Signed-off-by: Huong Nguyen <huongg1409@gmail> * hover style for copy button Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix padding for action bar Signed-off-by: Huong Nguyen <huongg1409@gmail> * first working version of box shadow Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix position and transition issue Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix the issue when selecting on the node again the second time Signed-off-by: Huong Nguyen <huongg1409@gmail> * set button heigh and position of the tooltip Signed-off-by: Huong Nguyen <huongg1409@gmail> * update slicing notification when metadata panel is closed Signed-off-by: Sajid Alam <[email protected]> * Update flowchart.js Signed-off-by: Sajid Alam <[email protected]> * remove line at end of notification bar Signed-off-by: Sajid Alam <[email protected]> * persist from node from user selection Signed-off-by: Sajid Alam <[email protected]> * box shadow using after class Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix the from node bug Signed-off-by: Huong Nguyen <huongg1409@gmail> * increase box shadow length Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix bug Signed-off-by: Huong Nguyen <huongg1409@gmail> * update buffer name Signed-off-by: Huong Nguyen <huongg1409@gmail> * update function name Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix merge commit Signed-off-by: Huong Nguyen <huongg1409@gmail> * separate action bar and notification Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove unused mixin Signed-off-by: Huong Nguyen <huongg1409@gmail> * include extra check for run command length Signed-off-by: Huong Nguyen <huongg1409@gmail> * include test for notification Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix bug on highlighting node list in slicing mode Signed-off-by: Huong Nguyen <huongg1409@gmail> * new animation for action bar Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix bug about the number of sliced pipeline Signed-off-by: Huong Nguyen <huongg1409@gmail> * add new props in test Signed-off-by: Huong Nguyen <huongg1409@gmail> * Feature/slicing pipeline add notification (#2003) * change order only when store updated Signed-off-by: Sajid Alam <[email protected]> * update tests Signed-off-by: Sajid Alam <[email protected]> * changes based on review Signed-off-by: Sajid Alam <[email protected]> * Set showSlicingNotification to true if a single node is clicked Signed-off-by: Sajid Alam <[email protected]> * Update flowchart.js Signed-off-by: Sajid Alam <[email protected]> * enable notification when single node is clicked Signed-off-by: Sajid Alam <[email protected]> * Delete sliced-pipeline-action-bar.test.js Signed-off-by: Sajid Alam <[email protected]> * add comments Signed-off-by: Sajid Alam <[email protected]> * merge Signed-off-by: Sajid Alam <[email protected]> --------- Signed-off-by: Sajid Alam <[email protected]> * highlight modularpipeline when collasped in slicing mode Signed-off-by: Huong Nguyen <huongg1409@gmail> * update slice button color Signed-off-by: Huong Nguyen <huongg1409@gmail> * removed unused props Signed-off-by: Huong Nguyen <huongg1409@gmail> * update name for slicePipeline Signed-off-by: Huong Nguyen <huongg1409@gmail> * update test with new name for SET_SLICE_PIPELINE Signed-off-by: Huong Nguyen <huongg1409@gmail> * get runCommand based on type of the element Signed-off-by: Huong Nguyen <huongg1409@gmail> * update run command to get from metadata Signed-off-by: Huong Nguyen <huongg1409@gmail> * add comments Signed-off-by: Huong Nguyen <huongg1409@gmail> * update test for sliced-pipeline Signed-off-by: Huong Nguyen <huongg1409@gmail> * update run-command test with new logic Signed-off-by: Huong Nguyen <huongg1409@gmail> * Disable slicing feature in embedded mode (#2072) * Disable slicing feature in embedded mode Signed-off-by: Jitendra Gundaniya <[email protected]> * test fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Name fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Moved visibleSlicing prop under visible Signed-off-by: Jitendra Gundaniya <[email protected]> * Test fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Remove unused code Signed-off-by: Jitendra Gundaniya <[email protected]> --------- Signed-off-by: Jitendra Gundaniya <[email protected]> * Telemetry/slicing pipeline (#2054) * adding data-test Signed-off-by: Huong Nguyen <huongg1409@gmail> * include getDataTestAttribute Signed-off-by: Huong Nguyen <huongg1409@gmail> --------- Signed-off-by: Huong Nguyen <huongg1409@gmail> Co-authored-by: Huong Nguyen <huongg1409@gmail> * fix the highlight of modular pipeline Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix the position of run command Signed-off-by: Huong Nguyen <huongg1409@gmail> * use 1.5 instead 2 for divided value Signed-off-by: Huong Nguyen <huongg1409@gmail> * Documentation: slicing pipeline in kedro-viz (#2043) * first draft Signed-off-by: Huong Nguyen <huongg1409@gmail> * fix typo Signed-off-by: Huong Nguyen <huongg1409@gmail> * include gif Signed-off-by: Huong Nguyen <huongg1409@gmail> * use uncapital words Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove the word just Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove # Signed-off-by: Huong Nguyen <huongg1409@gmail> * use verb instead of slicing Signed-off-by: Huong Nguyen <huongg1409@gmail> * use stable version Signed-off-by: Huong Nguyen <huongg1409@gmail> * include new gif and increase the size Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove un-used gif Signed-off-by: Huong Nguyen <huongg1409@gmail> * remove ./ in path Signed-off-by: Huong Nguyen <huongg1409@gmail> * revert back to previous syntax Signed-off-by: Huong Nguyen <huongg1409@gmail> * reuploaded gif files Signed-off-by: Huong Nguyen <huongg1409@gmail> * update text Co-authored-by: rashidakanchwala <[email protected]> Signed-off-by: Huong Nguyen <[email protected]> * update text Co-authored-by: rashidakanchwala <[email protected]> Signed-off-by: Huong Nguyen <[email protected]> * update text Co-authored-by: rashidakanchwala <[email protected]> Signed-off-by: Huong Nguyen <[email protected]> * update text Co-authored-by: rashidakanchwala <[email protected]> Signed-off-by: Huong Nguyen <[email protected]> --------- Signed-off-by: Huong Nguyen <huongg1409@gmail> Signed-off-by: Huong Nguyen <[email protected]> Co-authored-by: Huong Nguyen <huongg1409@gmail> Co-authored-by: rashidakanchwala <[email protected]> * update release note Signed-off-by: Huong Nguyen <huongg1409@gmail> --------- Signed-off-by: Huong Nguyen <huongg1409@gmail> Signed-off-by: Sajid Alam <[email protected]> Signed-off-by: Sajid Alam <[email protected]> Signed-off-by: Jitendra Gundaniya <[email protected]> Signed-off-by: Huong Nguyen <[email protected]> Co-authored-by: Huong Nguyen <huongg1409@gmail> Co-authored-by: Sajid Alam <[email protected]> Co-authored-by: Sajid Alam <[email protected]> Co-authored-by: Jitendra Gundaniya <[email protected]> Co-authored-by: rashidakanchwala <[email protected]>
Description
This feature will show a notification to kedro users to
Hold Shift + Click on another node to slice pipeline
. Moreover, the order in which nodes are selected to should not matter and be resolved Programatically.Development notes
When testing on your machine locally, or through gitpod, please note that this PR only covers the functionalities below:
Notification bar:
Checklist
RELEASE.md
file