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(connect-ui): handle generic auth method #2754

Merged
merged 5 commits into from
Sep 24, 2024
Merged

Conversation

bodinsamuel
Copy link
Collaborator

@bodinsamuel bodinsamuel commented Sep 23, 2024

Describe your changes

Contributes to https://linear.app/nango/issue/NAN-1703/create-ui

  • Handle generic auth method
    API Key, Basic auth, unauth, and all auth that do not take a connection config (e.g: google drive). If your integration is requiring a connectionConfig it's not yet supported

  • Frontend: type errors + fix a typo

  • Frontend: add a way to clear SDK's state, it was annoying to do that manually when debugging

  • Frontend: slightly change the way the modal is being created
    We were creating an empty popup and then changing the URL which made the catching of popup blocker more random

🧪 Tests

npm i
npm run dw
npm run dwa

cd packages/connect-ui
touch .env
> VITE_LOCAL_SECRET_KEY=FILL
> VITE_LOCAL_PUBLIC_KEY=FILL
> VITE_LOCAL_HOSTNAME=http://localhost:3003


npm run connect-ui:dev:watch
Screen.Recording.2024-09-23.at.16.19.41.mov

@bodinsamuel bodinsamuel self-assigned this Sep 23, 2024
Copy link

linear bot commented Sep 23, 2024

NAN-1703 Create UI

};

if (this.status === AuthorizationStatus.BUSY) {
const error = new AuthError('The authorization window is already opened', 'windowIsOppened');
const error = new AuthError('The authorization window is already opened', 'windowIsOpened');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixing this typo might be a breaking change

reject(error);
return;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we were not rejecting but not returning, not sure if it was intentional but if yes it was annoying to get an error but still get a popup. Happy to discuss

const creationDateA = a.creationDate || new Date(0);
const creationDateB = b.creationDate || new Date(0);
return creationDateB.getTime() - creationDateA.getTime();
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Happy to discuss this sort, I'm sure it was intentional but very confusing to have this "random" sort in the UI

Copy link
Collaborator

Choose a reason for hiding this comment

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

I imagine it was to show the integrations by most recently added in the dashboard. The sorting can be done client side I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, actually that's what I'm changing because it's super annoying to have seemingly random order in the dashboard.
Maybe @khaliqgant can clarify what were the expectations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

merging, happy to revert if needed

packages/connect-ui/src/lib/nango.ts Show resolved Hide resolved

this.status = AuthorizationStatus.IDLE;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that calling clear in nango.auth() will solve this bug: https://linear.app/nango/issue/NAN-1773/fix-the-authorization-window-is-already-opened

const creationDateA = a.creationDate || new Date(0);
const creationDateB = b.creationDate || new Date(0);
return creationDateB.getTime() - creationDateA.getTime();
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I imagine it was to show the integrations by most recently added in the dashboard. The sorting can be done client side I think.

@bodinsamuel bodinsamuel enabled auto-merge (squash) September 24, 2024 15:45
@bodinsamuel bodinsamuel merged commit 3920a17 into master Sep 24, 2024
22 checks passed
@bodinsamuel bodinsamuel deleted the feat/connect-ui-auth branch September 24, 2024 15:53
hassan254-prog pushed a commit that referenced this pull request Sep 25, 2024
## Describe your changes

Contributes to https://linear.app/nango/issue/NAN-1703/create-ui

- Handle generic auth method
API Key, Basic auth, unauth, and all auth that do not take a connection
config (e.g: google drive). If your integration is requiring a
connectionConfig it's not yet supported

- Frontend: type errors + fix a typo
- Frontend: add a way to clear SDK's state, it was annoying to do that
manually when debugging
- Frontend: slightly change the way the modal is being created
We were creating an empty popup and then changing the URL which made the
catching of popup blocker more random


## 🧪 Tests
```ts
npm i
npm run dw
npm run dwa

cd packages/connect-ui
touch .env
> VITE_LOCAL_SECRET_KEY=FILL
> VITE_LOCAL_PUBLIC_KEY=FILL
> VITE_LOCAL_HOSTNAME=http://localhost:3003


npm run connect-ui:dev:watch
```


https://github.com/user-attachments/assets/080d3746-bc1e-457e-a80b-175646fc63a4
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.

2 participants