Feat: Enhance import process #2728
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
|
Questions to reviewer:
|
mstykow
requested changes
Jan 29, 2025
There was a problem hiding this comment.
general comments:
- great first PR! it's clear you've thought deeply about the problem and addressed many edge cases.
- this PR is too large. it's intimidating to review it, you are getting feedback only after days of work, and it will take a long time to get it to a mergable state because it changes so many things. next time, please think about how to chop your changes into smaller chunks.
PhilippMa
force-pushed
the
feat/enhance-import-process
branch
from
93fa729 to
778fa5e
Compare
* Other input file formats will be handled through a separate import workflow
* this function will be needed by the "Import File" menu
* click behavior for menu item is still WIP
* WIP: handling of this channel in the frontend still needs to be implemented
* triggered by menu action * allows selection of input file * selection of output file, verification of file paths and the actual file import will follow in future commits
* needed to separate void and non-void callbacks so that non-void callbacks can return null on error
* had to change popup handling to allow for passing props to popups
* can be used for both selecting the input file and the location of the created opossum file
* second line is autofilled based on the selected input path
* give more specific error messages for empty path or invalid file extension
* delete FileSupportPopup * delete import logic from open handlers * delete some unused IPC channels including handlers/send functions
* this check is performed on edit for the paths, as it should not be an expensive operation
* more precise typing gives earlier/better feedback when listeners don't fit the type expected by the ipc handler
… to log display * not using form helper texts is more consistent with other forms in OpossumUI * only validating on import drastically reduces the state variables necessary in import dialog * to still get file overwrite warnings, force users to select file paths through system dialogs
PhilippMa
force-pushed
the
feat/enhance-import-process
branch
from
778fa5e to
10076cb
Compare
mstykow
requested changes
Jan 31, 2025
* as the textbox is no longer editable, there is no need to have a second clickable element anymore
mstykow
requested changes
Feb 3, 2025
mstykow
requested changes
Feb 3, 2025
mstykow
requested changes
Feb 3, 2025
* pass customInput to TextBox in order to disable unwanted TextField behavior
* change TextBox to no longer use deprecated props
mstykow
approved these changes
Feb 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Added an import dialog for importing non-.opossum files into OpossumUI. The import dialog is accessible through a new "Import file" menu item with submenus for all supported file types. In the dialog, users select the file to import and the location where the newly created .opossum file should be saved, either through native file select dialogs or through text field input. The file paths are validated, invalid file paths cause error messages to show and the import button only works when the file paths are valid. Clicking the import button uses the existing functions for loading files into OpossumUI, but the loading log information is displayed directly in the import dialog instead of the ProcessPopup used when opening files. As non-.opossum files are now handled through this import workflow, the open file action now only allows selection of .opossum files.
Context and reason for change
We want OpossumUI to be able to import files with multiple different formats. For this purpose, separating importing from just plainly opening files creates more clarity for the user. Also, it was deemed confusing that the existing workflow for opening legacy opossum files displayed a warning over having to create a new .opossum file, and that it would be better to allow the user to select the location of this new .opossum file instead.
In future issues, the import dialog will be extended with supporting more file types and handling errors during file conversion.
How can the changes be tested
For viewing that the loading logs are displayed correctly, either a large file should be imported or some manual delays put into the loadInputAndOutputFromFilePath function in importFromFile.ts, as small files are loaded so quickly that the loading logs are only visible for a very short time before the dialog closes.
closes #2714