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

feat: add loading state for import actions #4217

Merged
merged 5 commits into from
Sep 23, 2024

Conversation

HelixY2J
Copy link
Contributor

@HelixY2J HelixY2J commented Jul 28, 2024

This PR addresses the need for a loading state in the Import/Export modals that includes collections and environments in scope.

Closes #4145 HFE-594.

What's changed

  • Introduce state variables to keep track of the loading status during a network call while importing to the data store applicable to the team workspace. The handleImportToStore function promise resolution is awaited.
  • Introduce a loading prop to the FileImport & UrlImport components that default to false - toggles the Import CTA loading state accordingly. The import CTA stays disabled during a loading state.
  • Adds a loading prop to the MyCollectionImport component to handle the loading state with import from personal collections flow while at a team workspace.
  • The FileSource() & UrlSource() functions now include a new isLoading parameter via which the above state variables are supplied from the respective component.

Note to reviewers

Please verify the import collections/environments flow e2e.

Copy link
Member

@jamesgeorge007 jamesgeorge007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • All the relevant importer types (HoppPostmanImporter, HoppInsomniaImporter and HoppOpenAPImporter) need to be updated similar to the updates proposed to HoppRESTImporter:

    • Remove the try/finally block.
    • Wait for the promise from handleImportToStore() to resolve.
    • Add a new metadata entry isLoading.
  • Since we're adding a new isLoading metadata field for the FileSource helper function, we'll have to make the following changes to its source.

import FileImportVue from "~/components/importExport/ImportExportSteps/FileImpor
import { defineStep } from "~/composables/step-components"
 
 import { v4 as uuidv4 } from "uuid"
+import { Ref } from "vue"
 
 export function FileSource(metadata: {
   acceptedFileTypes: string
   caption: string
   onImportFromFile: (content: string[]) => any | Promise<any>
+  isLoading?: Ref<boolean>
 }) {
   const stepID = uuidv4()
 
export function FileSource(metadata: {
     acceptedFileTypes: metadata.acceptedFileTypes,
     caption: metadata.caption,
     onImportFromFile: metadata.onImportFromFile,
+    loading: metadata.isLoading?.value,
   }))
 }
  • A new prop loading has to be added for the FileImport component and the import CTA that is rendered via HoppButtonPrimary needs to be supplied a loading prop attaching to the newly added prop. Also, the disable logic should be updated to account for the loading state.

<HoppButtonPrimary
class="w-full"
:label="t('import.title')"
:disabled="!hasFile || showFileSizeLimitExceededWarning"
@click="emit('importFromFile', fileContent)"
/>

  • We'll also need to account for Import from Personal collections represented by the HoppMyCollectionImporter importer. A new reactive state variable has to be added to track the loading state. The MyCollectionImport component will require updates similar to the FileSource component.

@jamesgeorge007 jamesgeorge007 changed the title Added loading state for importing collection modals feat: add loading state for import collections modal Aug 1, 2024
@jamesgeorge007 jamesgeorge007 changed the base branch from main to next September 11, 2024 08:49
@jamesgeorge007 jamesgeorge007 changed the title feat: add loading state for import collections modal feat: add loading state for Import/Export modals Sep 11, 2024
@jamesgeorge007 jamesgeorge007 changed the title feat: add loading state for Import/Export modals feat: add loading state for import actions Sep 11, 2024
Copy link
Member

@nivedin nivedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add loading state for gist and har importer as well

@jamesgeorge007
Copy link
Member

We can add loading state for gist and har importer as well

Accounted for OpenAPI import via URL step as well.

Copy link
Member

@nivedin nivedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

@AndrewBastin AndrewBastin merged commit 1701961 into hoppscotch:next Sep 23, 2024
1 check passed
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.

4 participants