[ui-importer] Add configured filesystems in importer #4131
Conversation
|
✅ Test files were modified. Ensure that the tests cover all relevant changes. ✅ |
nidhibhatg
force-pushed
the
nidhi_importer_filesystems
branch
from
465c3d9 to
42dccf4
Compare
Python Coverage Report •
Pytest Report
|
| ...i18n | ||
| }: FileChooserModalProps): JSX.Element => { | ||
| const { t } = i18nReact.useTranslation(); | ||
| const { cancelText = t('Cancel'), submitText = t('Submit') } = i18n; | ||
| const [destPath, setDestPath] = useState<string>(sourcePath); | ||
| const [submitLoading, setSubmitLoading] = useState<boolean>(false); | ||
| const [searchTerm, setSearchTerm] = useState<string>(''); | ||
| const [showCreateFolderModal, setShowCreateFolderModal] = useState<boolean>(false); | ||
| const [polling, setPolling] = useState<boolean>(false); |
There was a problem hiding this comment.
We can avoid this state by using filesToUpload.length if its zero then pooling is false else true.
| }} | ||
| /> | ||
| )} | ||
| <InputModal |
There was a problem hiding this comment.
Can we refactor the **/StorageDirectoryActions/CreateAndUpload/CreateAndUploadAction.tsx component to take the create file and folder action to separate component and reuse it here?
desktop/core/src/desktop/js/apps/storageBrowser/FileChooserModal/FileChooserModal.tsx
Outdated
Show resolved
Hide resolved
| const tableData: FileChooserTableData[] = useMemo(() => { | ||
| if (!filesData?.files) { | ||
| if (loading || error || !filesData?.files) { |
There was a problem hiding this comment.
Is this required to check it?
In case of loading and error, LoadingErrorWrapper will take care of showing the appropriate spinner or alert.
| @@ -34,9 +42,45 @@ interface ImporterSourceSelectorProps { | |||
| const ImporterSourceSelector = ({ setFileMetaData }: ImporterSourceSelectorProps): JSX.Element => { | |||
| const { t } = i18nReact.useTranslation(); | |||
| const uploadRef = useRef<HTMLInputElement>(null); | |||
| const [showFileChooserModal, setShowFileChooserModal] = useState<boolean>(false); | |||
| const [filePath, setFilePath] = useState<string>(''); | |||
There was a problem hiding this comment.
Instead of two states, one for opening the fileChooserModal and one to set the userHomeDirectory for the fileChooserModal. Can we use [selectedUserHomeDirectory, setSelectedUserHomeDirectory] and, based on whether this is undefined or not, we can show the FileChooserModal?
| loading, | ||
| error, | ||
| reloadData | ||
| } = useLoadData<FileSystem[]>(FILESYSTEMS_API_URL); | ||
| const { save: upload } = useSaveData<LocalFileUploadResponse>(UPLOAD_LOCAL_FILE_API_URL); |
There was a problem hiding this comment.
Can you add error handling for uploading the local file as well?
desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx
Outdated
Show resolved
Hide resolved
| {t('Select a source to import from')} | ||
| </div> | ||
| <div className="hue-importer__source-selector-options"> | ||
| {(window as hueWindow).ENABLE_DIRECT_UPLOAD && ( |
There was a problem hiding this comment.
It will be a good idea to extract this code to a new component, as it will move all the code related to a local file to one place, and this component will look a bit cleaner.
| @@ -124,10 +156,61 @@ const FileChooserModal = ({ | |||
| if (record.type === BrowserViewType.dir) { | |||
| setDestPath(record.path); | |||
| } | |||
| if (isImport && record.type === BrowserViewType.file) { | |||
There was a problem hiding this comment.
The isImport keyword is too specific to our use case, we can make this functionality generic. Something like isFileSelectionAllowed or anything more generic
|
Lots of good feedback from @ramprasadagarwal. I'll hold while you guys sort it out, but I did notice that the screenshot of the drag and drop looks weird where the "drop area" appears behind the action buttons of the modal. |
desktop/core/src/desktop/js/apps/newimporter/ImporterSourceSelector/ImporterSourceSelector.tsx
Outdated
Show resolved
Hide resolved
| @@ -14,14 +14,22 @@ | |||
| // See the License for the specific language governing permissions and | |||
| // limitations under the License. | |||
|
|
|||
| import React, { useRef } from 'react'; | |||
| import React, { useRef, useState } from 'react'; | |||
There was a problem hiding this comment.
Please add the tests for this file.
nidhibhatg
force-pushed
the
nidhi_importer_filesystems
branch
from
811ebb9 to
3700a3c
Compare
What changes were proposed in this pull request?
How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.