Skip to content
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

Added File Browser / File Tree panel #2460

Merged
merged 20 commits into from Feb 12, 2024

Conversation

andrewmurraydavid
Copy link
Contributor

@andrewmurraydavid andrewmurraydavid commented Feb 8, 2024

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:

image

image

@breiler
Copy link
Collaborator

breiler commented Feb 8, 2024

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
Copy link
Contributor Author

andrewmurraydavid commented Feb 8, 2024

Thanks for the review, @breiler!

  1. I will move the FileBrowserPanel away from ugs-core. I'm still getting familiar with the codebase and short explanation was very helpful.
  2. I will update the code to use the OpenFileAction. Thanks for the recommendation!
  3. The localization example is very useful! Thank you!
  4. 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
Copy link
Contributor Author

andrewmurraydavid commented Feb 8, 2024

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
Copy link
Contributor Author

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

@andrewmurraydavid
Copy link
Contributor Author

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

@breiler
Copy link
Collaborator

breiler commented Feb 12, 2024

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

@andrewmurraydavid
Copy link
Contributor Author

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
Copy link
Contributor Author

Both comments have been addressed. Thank you!

@breiler breiler merged commit f06322c into winder:master Feb 12, 2024
2 checks passed
@andrewmurraydavid andrewmurraydavid deleted the file-browser branch February 12, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants