Added File Browser / File Tree panel

andrewmurraydavid commented 5 months ago

Description

Added a File Browser panel, similar to how most CNCs with control boards have file lists.

The purpose of this is to make it easier for the user to load files.

Current issues:

I could not find an easy way to trigger the Editor to open when a file is loaded from the File Tree, even though I am using GUIHelpers.openGcodeFile(gcodeFile, backend); like it's used in the MainWindow.
When folders are expanded and the clicks "Show hidden", the folders end up collapsed (not sure if that would need to be persistant)
Couldn't find a way to localize the menu text and the action text.

Note to reviewer: please let me know what other issues are found and which of the issues i mentioned need addressing.

Screenshots:

breiler commented 5 months ago

First of all, thanks for digging into UGS. =)

I have a couple of things that I would like to tweak that I think will make things easier in the long run.

The ugs-core module should not depend on netbeans. So move the FileBrowserPanel to the somewhere in ugs-platform-ugscore module, for instance here. I'd prefer this feature to be a separate ugs-plugin but it can be moved in a later stage. Only put things in the ugs-core if it is needed by many plugins or needed in both classic and platform edition.

I think that you should disable the panel if the machine is running. Otherwise it could technically be possible to load a new file while the machine is running.

File loading is handled a bit differently in the platform edition. The easiest way is probably to instanciate and execute this action.

Translations of actions and TopComponent can be a bit tricky, the most robust way is probably this: https://github.com/winder/Universal-G-Code-Sender/blob/master/ugs-platform/ugs-platform-welcome-page/src/main/java/com/willwinder/ugp/welcome/WelcomePageTopComponent.java#L103

andrewmurraydavid commented 5 months ago

Thanks for the review, @breiler!

I will move the FileBrowserPanel away from ugs-core. I'm still getting familiar with the codebase and short explanation was very helpful.
I will update the code to use the OpenFileAction. Thanks for the recommendation!
The localization example is very useful! Thank you!
Good call on disabling the file loading from the file browser/tree when the machine is running! I will add that!

Question: naming-wise, which makes more sense for this codebase: FileTree or FileBrowser? Asking because I noticed there are some FileBrowser references in the code already, and don't want to create ambiguity with my code change.

I should have some time to implement these changes tonight after work.

andrewmurraydavid commented 5 months ago

So when trying to move the FileBrowserPanel class in the ugs-platform-ugscore, the WidgetPreviewer seems to require making a dependency on the ugs-platform-ugscore module.

Idea is smart enough to catch possible circular dependencies and gives this warning:

Adding dependency on module 'ugs-platform-ugscore' will introduce circular dependency between modules 'ugs-core' and 'ugs-pendant'. Add dependency anyway?

Any suggestions? Or I can just ignore the WidgetPreviewer, as I'm guessing that might be legacy?

andrewmurraydavid commented 5 months ago

@breiler this should be back in review. Thank you for your time to look over it!

andrewmurraydavid commented 4 months ago

@breiler let me know if there's anything I'm missing. Thanks!

breiler commented 4 months ago

See the two comments I made in the review, the first one I think must be fixed before merging.

andrewmurraydavid commented 4 months ago

This seems to be the only comment I can see. Are you talking about moving the panels away from core? That was done after the review and now thy live here: ugs-platform/ugs-platform-ugscore/src/main/java/com/willwinder/ugs/nbp/core/ as per your comment.

First of all, thanks for digging into UGS. =)

I have a couple of things that I would like to tweak that I think will make things easier in the long run.

The ugs-core module should not depend on netbeans. So move the FileBrowserPanel to the somewhere in ugs-platform-ugscore module, for instance here. I'd prefer this feature to be a separate ugs-plugin but it can be moved in a later stage. Only put things in the ugs-core if it is needed by many plugins or needed in both classic and platform edition.

I think that you should disable the panel if the machine is running. Otherwise it could technically be possible to load a new file while the machine is running.

File loading is handled a bit differently in the platform edition. The easiest way is probably to instanciate and execute this action.

Translations of actions and TopComponent can be a bit tricky, the most robust way is probably this: https://github.com/winder/Universal-G-Code-Sender/blob/master/ugs-platform/ugs-platform-welcome-page/src/main/java/com/willwinder/ugp/welcome/WelcomePageTopComponent.java#L103

andrewmurraydavid commented 4 months ago

Both comments have been addressed. Thank you!

winder / Universal-G-Code-Sender