Disable actions when deleting #5775
Conversation
Disable buttons to avoid user to perform actions on a deleting object Signed-off-by: Thiago Mendes <tzig@tutanota.de>
trmendes
requested review from
jeffmaury and
feloy
and removed request for
a team
|
When I delete images and open the contextual menu on the image actions, I can see that some entries are still enabled (most probably entries added by extensions). Do you think it could be posisble to disable them, or disable the three dots instead? |
Hi @feloy. I have noticed it too! Tomorrow I will try to check how to disable the entries added by extensions. |
|
I'm a little uneasy about this - for our own actions we're changing their enablement based on state, but for extensions we're just saying 'no you can't do that here'. I suppose it is ok for deletion b/c the object is going away anyway, but I worry it's removing the onus on extension developers to correctly enable their actions based on state (status in this case). |
I agree with @deboer-tim . What I have in mind for trying tomorrow is to have all other actions disabled for the deletion only. |
|
|
||
| test('Expect buttons to be disabled when deleting', async () => { | ||
| const { component } = render(ContainerActions, { container }); | ||
| component.$on('update', updateMock); |
There was a problem hiding this comment.
Shouldn't need this line (since we're not doing anything with the mock).
|
|
||
| test('Expect buttons to be disabled when deleting', async () => { | ||
| const { component } = render(DeploymentActions, { deployment }); | ||
| component.$on('update', updateMock); |
There was a problem hiding this comment.
Shouldn't need this line either.
|
|
||
| test('Expect buttons to be disabled when deleting', async () => { | ||
| const { component } = render(ServiceActions, { service }); | ||
| component.status = 'DELETING'; |
There was a problem hiding this comment.
I thought it was suspicious that the test still worked when the status was set on component instead of service ;-) and indeed this test is still passing because the button is already in progress from the previous test - if you reorder the tests, it fails.
I didn't have enough time to figure out why the tests aren't independent, but needs some cleanup between. To confirm the test is working it's probably good to confirm that it fails when this line is deleted and not otherwise.
There was a problem hiding this comment.
Ops! Thank you for noticing that :D.
| test('Expect buttons to be disabled when deleting', async () => { | ||
| const { component } = render(ServiceActions, { service }); | ||
| component.status = 'DELETING'; | ||
| component.$on('update', updateMock); |
There was a problem hiding this comment.
Shouldn't need this line either.
check if buttons are disabled when deleting objects Signed-off-by: Thiago Mendes <tzig@tutanota.de>
trmendes
force-pushed
the
5739_disable_actions_when_deleting
branch
from
41b4b5b
to
b5b4ba9
Compare
|
Hi @deboer-tim. Thanks for the hints. I wonder if I'm missing something. |
@deboer-tim, @feloy: I'm leaving the 'disable menu' functionality for further discussions and, to another Issue/PR. What are your comments about? The PR is open for another review :D. I hope we are able to merge it soon. |
|
@feloy @deboer-tim looks like you didn't reply to @trmendes comments switching to draft |
What does this PR do?
To avoid user to perform any action on an object that is being deleted, this PR disables some actions buttons when user click on delete.
Screenshot / video of UI
delete.mp4
What issues does this PR fix or reference?
Fixes #5739
How to test this PR?
Option 1) Unit Tests
Option 2) Play around with creating and deleting. Expect buttons to be disabled.