-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
3865-Add-Integrations #3870
3865-Add-Integrations #3870
Conversation
@FelixMalfait I'm done with the design. I have coded constant in such a way that in future new integrations can be easily added. I have two doubts:
|
That's great as always @Kanav-Arora thanks!
What I meant was that the examples I think we should have were the one I added in my screenshot (cal.com, Slack, Mailchimp, Tally) |
@FelixMalfait Sorry now I get it, so from where should I download images as these brand logos are pixelated? I'll change the integrations. |
@FelixMalfait |
@Kanav-Arora you may find it here: Thanks! |
Hi @FelixMalfait |
Thanks! It's still the big Windmill logo |
packages/twenty-front/src/pages/settings/integrations/constants/Zapier.ts
Show resolved
Hide resolved
@FelixMalfait done |
Hi @FelixMalfait |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, good work as always @Kanav-Arora :)
@Kanav-Arora I'm merging it. As a follow up, could you introduce a featureFlag (similar to IS_MESSAGING_ENABLED) and use it to hide integration link in the Settings navigation? |
@charlesBochet why would we want to keep it behind a feature flag? Zapier app has been published |
Also as follow up here is a list of small things to adjust:
Other than that it looks great! |
@Kanav-Arora I have actually added the featureFlag in the last commit so we can merge it! |
Closes #3865
I have coded constant in such a way that in future new integrations can be easily added.