-
Notifications
You must be signed in to change notification settings - Fork 350
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
Disable toolbar buttons on empty pipeline editor #741
Disable toolbar buttons on empty pipeline editor #741
Conversation
{ action: 'undo', label: 'Undo' }, | ||
{ action: 'redo', label: 'Redo' }, | ||
{ action: 'cut', label: 'Cut' }, | ||
{ action: 'copy', label: 'Copy' }, | ||
{ action: 'paste', label: 'Paste' }, | ||
{ action: 'addComment', label: 'Add Comment', enable: true }, | ||
{ action: 'delete', label: 'Delete', enable: true }, | ||
{ action: 'delete', label: 'Delete' }, |
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.
We don't need to set the enable
property on these commands, they're handled by context on canvas already
* | ||
* @param pipelineDefinition | ||
*/ | ||
static isEmptyCanvas(pipelineDefinition: any): boolean { |
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.
There might be cases we want to check if canvas is completely clean of nodes and comments in order to enable/disable action buttons (eg. we only disable clean pipeline button if canvas is empty and there is nothing else left to clean)
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.
@karlaspuldaro not sure if this is a known issue.
when adding/removing nodes/links the Undo/Redo buttons are enabled/disabled accordingly and work as expected. now when you clear the pipeline you get a confirmation dialog with the message (You can not undo this.) which is good. after accepting the confirmation and the pipeline is cleared the Undo button is enabled (i would expect it disabled since you cannot undo the clearing). you can click on the Undo but it doesn't do anything (as expected). however, at that point the Redo becomes enabled and clicking on it doesn't do anything either. in the end you get to a point where Undo/Redo buttons are enabled and you can click on them although you cannot really undo/redo any action.
Listen to undo/redo/delete/addComment commands Handle empty canvas case (no nodes or comments) Update enable parameters of toolbarConfig
11585e1
to
d1dee70
Compare
@vabarbosa the undo/redo issue when clearing a pipeline is existing, looks like we never noticed it, or never properly handled it. I opened #756 where we can investigate as a separate PR, as discussed today. |
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.
I'm still playing with this locally but here's my code review for now
I tested locally things seem to work great, I did open #757 with separate issues I found while testing this. I also ran the ui tests locally and they passed. I also check and this looks good in dark mode as well (except of course my previously mentioned issue). So once my previous code comments are address this LGTM |
Simplify behaviour of arrangeHorizontally-Vertically buttons Rename isNewPipeline() to isEmptyPipeline()
@karlaspuldaro I see approvals coming in for this pr. Are we ready to remove the |
@lresende I wanted to add more tests for this but that can be done later, we already have basic tests up and a note for extra ones |
Fixes #691
Toolbar buttons enabled on a populated pipeline

Toolbar buttons enabled on empty pipeline

We are also handling cases of pipeline with no nodes but comments are left (user can still clear pipeline and arrange items)

Developer's Certificate of Origin 1.1