Add support for delete hotkey and multiple deletes in browsers

[sebjulliand]

Changes

This PR adds support for pressing the delete key to delete selected items in the Connection, IFS and Object browser.
It also restore the possibility to delete source files (this was lost with the filter enhancement).

Multiple elements can be deleted at once.

All delete confirmation dialogs are now modals.

Checklist

• have tested my change

[chrjorgensen]

@sebjulliand Nice enhancement! 👍 I love keyboard navigation, so this is a very welcome enhancement, although I'm not deleting as much as other operations... 😉 😍

A quick test seems to show some navigation issues:

• In Connection Browser (CB) and Object Browser (OB), when an item is marked and the cursor moved to another item, it's not the item marked by the cursor that will be deleted, but the item selected (filled background). This can be confusing for the user (me!).
• Delete connection confirmation dialog is not modal, but still a warning message.
• In the OB Delete does not work if no object is selected. I would expect the object marked by the cursor to be deleted when pressing Delete.
• How do I select a member in OB without it being opened?
• It's not possible to mark objects in OB as in IO - not enabled for OB, since there are no drag'n'drop?
• Pressing Delete in IO without marking any files shows the following error:

I hope the navigation issues can be fixed by having Space mark an item and Enter open the marked item(s). It's not that intuitive to have to hold Shift and press Up or Down to mark one or more items in the lists... But maybe it's not possible in the current API's? 🙏

[chrjorgensen]

@worksofliam I'm looking forward to your review - I think we all should have a try with this PR, since it's about deleting objects and we should reduce the risk of users deleting something by mistake... 🙏
@Wright4i Your opinion is also welcome, of course! ❤️

[chrjorgensen]

Some additional thoughts:

• maybe we should have all views working like the IFS browser, i.e. allowing multiple selections (drag'n'drop may be overkill, but could be used for re-arranging the order of the items)?
• It should be clear to the user what is going to be deleted when Del is pressed. If not possible to make completely clear, then maybe remove the Delhotkey and have the user right-click instead?

I like the way the delete operation has been changed in the code. So maybe we at least should use that change?

FInally, is it just me or does Code for IBM i start faster now, with the new initialization of the Connection Browser? 👍

[sebjulliand]

Thank you @chrjorgensen , on point as usual!
I fixed the few issues you found I could work on. The rest (mostly the selection bits) is in the hand of the API and I have no way of working around that, which makes the delete key support questionable. I agree the other enhancement should be kept. The hotkey is up to debate 😁 Multi-selection/drag and drop should also be considered.

[chrjorgensen]

Agree - let's see what the other team members @worksofliam and @Wright4i have to say...

[worksofliam]

Nice PR. Love the UX feature. It might be easier for me to go through and delete all my testing connections now - thanks!

Just some thoughts:

• How do you use the delete key to delete a connection? When you highlight/select a connection, you connect right away.
• You can also apply that logic to the IFS Browser and Object Browser. You can use the delete key without clicking it first (which tends to expand or connect) - though it is still useful.

edit: i realize this is what you and @chrjorgensen were already discussing! Perhaps for the connection, we require them click on the connection button, though this is a huge UX change and likely not to be appreciated by many. But in general, I say key the functionality for IFS Browser and Object Browser.

[worksofliam]

Long term goal, not required here: get this DLTOBJ into an API. We have checkObject, why not deleteObject?

[worksofliam]

@sebjulliand Something I just found out:

• In the IFS Browser you can shift click on any node without it triggering an expand or open. We have multi-select allowed here, which in theory makes the delete key fit well with this.
• In the Object Browser, shift clicking makes no difference which kind of makes the delete key silly since you have to expand/open to delete. The Connection Browser has the same UI logic as the Object Browser.
• I love that the delete key works on Objects and Streamfiles.

[sebjulliand]

@chrjorgensen , @worksofliam : round 2!

• I cleaned up the branch
• Made the deletion dialog display logic consistent between the browsers
• Connection browser supports multiple selection to delete several connections at once
• One command is responsible for deleting the items in the object browser. The items are responsible for providing the logic to delete them. I tried to make the post-delete refresh as optimized as possible.
• Protected/readonly items cannot be deleted
• The protected flag was not set or passed along correctly

[sebjulliand]

@chrjorgensen I'll need your translation skills too! 😁

[chrjorgensen]

@sebjulliand Could you add the number of elements to the confirmation messages - like it is in the IFS browser? I think that is an extra assurance for the user to see the number of elements that will be deleted - if 3 was supposed to be selected and 4 is mentioned in the message, this is a quick alert.

[sebjulliand]

@sebjulliand Could you add the number of elements to the confirmation messages - like it is in the IFS browser? I think that is an extra assurance for the user to see the number of elements that will be deleted - if 3 was supposed to be selected and 4 is mentioned in the message, this is a quick alert.

There you go!

[chrjorgensen]

@sebjulliand This error is occurring when I start debugging:

[{
	"resource": "/d:/Data/Develop/code-for-ibmi/src/views/objectBrowser.ts",
	"owner": "typescript",
	"code": "2663",
	"severity": 8,
	"message": "Cannot find name 'isProtected'. Did you mean the instance member 'this.isProtected'?",
	"source": "ts",
	"startLineNumber": 324,
	"startColumn": 105,
	"endLineNumber": 324,
	"endColumn": 116
}]

Maybe your branch was not updated to latest master?

[chrjorgensen]

@sebjulliand Something's wrong with the current state - when I check out your feature branch, I get the following error:

Could you fix the merge conflict present and force-push your local branch (having no errors!)?
Would be nice to complete this great change asap. 😍

[sebjulliand]

@chrjorgensen I had to nuke the branch and make everything clean, so I had to force-push a clean state, hence Git being confused.
I fixed the conflict so now it should be OK - pulling the changes shouldn't show any error. And hopefully it still works 🤣

[worksofliam]

Bug: I am getting this message when expanding an *ALL object filter - but worked fine with *SRCPF:

Deleting multiple connections works like a charm:

Deleting multiple directories works, but I think a progress indicator would be nice instead of messages for each chosen path, and then perhaps a final message saying they have been deleted. Not a requirement but nice for the user.

Deleting filters worked good:

[sebjulliand]

Thanks for the testing guys.
@worksofliam I fixed my stupid mistake. I also went the extra mile and optimized the delete process for the IFS files:

• The selection is first reduced to keep only folders and files whose parent folders are not in the selection
• We build a single rm -rf command for what remains after the reduction
• The command is run with a progress notification

It goes faster than running rm for each element, that goes without saying 😋

@chrjorgensen I'll need once again a small translation 🙏🏻

[worksofliam]

It liked me deleting an object and filter at the same time - nice!

And worked with the IFS Browser!

Plus - the progress bars are working as expected!

[chrjorgensen]

@sebjulliand Works great! 😍

Only one issue found: Deleting members shows this confirmation message - with a ()?

[sebjulliand]

@sebjulliand Works great! 😍

Only one issue found: Deleting members shows this confirmation message - with a ()?

This is supposed to be the member's extension between ()! Does your member has no extension?!

[chrjorgensen]

@sebjulliand Works great! 😍
Only one issue found: Deleting members shows this confirmation message - with a ()?

This is supposed to be the member's extension between ()! Does your member has no extension?!

No, no extension... but I think it is quite inconsistent to present a member as LIB/FILE/MEMBER (EXT), when we in all other places write LIB/FILE/MEMBER.EXT. WDYT? Would it be much work to present members like in all other places?

[chrjorgensen]

I made a silly - but valid - test, deleting 500 members in a source file... and found two issues:

1. The confirmation dialog filled the screen from top to bottom, with a slider. Is it possible to have a max height of the window?
2. The progress during deletion does not move - only the member name is rolling... Could it be the number to be deleted, that makes no progress move?

Sorry for being a PIA! 😢

[sebjulliand]

I made a silly - but valid - test, deleting 500 members in a source file... and found two issues:

1. The confirmation dialog filled the screen from top to bottom, with a slider. Is it possible to have a max height of the window?

2. The progress during deletion does not move - only the member name is rolling... Could it be the number to be deleted, that makes no progress move?

Sorry for being a PIA! 😢

1. We have no control over the dialogs' size. Either we keep it like that, or I can limit the number items listed and print "XX more..." if the list length goes beyond that limit.
2. I' look into that.

[sebjulliand]

@chrjorgensen : I fixed the progress stuck at 0 (rounding made the increment equals to 0...that was silly of me as the progress supports non-integer increments).
It should be OK now!

[chrjorgensen]

@sebjulliand Looking good to me - merge at will. 😍