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 integration page #1584

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Salman2301
Copy link
Contributor

Description 📣

This PR add environment, integration and sorting dropdown for the integration page.

Type ✨

  • Bug fix
  • New feature
  • Breaking change
  • Documentation

Tests 🛠️

output_video.mp4

@Salman2301
Copy link
Contributor Author

I would like to add the integration as an image rather than label. It saves horizontal space that need and aslo looks good imo
Screenshot 2024-03-18 at 5 25 23 PM

@Salman2301
Copy link
Contributor Author

Added tooltip
Screenshot 2024-03-18 at 8 59 17 PM

We can remove this

const integrationSlugNameMapping: Mapping = {

@Grraahaam
Copy link
Contributor

Hey @Salman2301! I like the idea, but I find the icons too big and I think it could look better if they were about half their size, wdyt?

And the transparent Github icon looks odd, maybe we should replace the transparent octocat with a white one?

@Salman2301
Copy link
Contributor Author

Salman2301 commented Mar 18, 2024

@Grraahaam agree with you on Github icon
Try half the size but it's feels too small
Screenshot 2024-03-19 at 5 19 11 AM
Try w-8 (32px) instead of w-12, I think this looks better
Screenshot 2024-03-19 at 5 19 26 AM

Open to suggestion

@Salman2301
Copy link
Contributor Author

With white color
Screenshot 2024-03-19 at 5 30 44 AM

@Grraahaam
Copy link
Contributor

That's better, but when infisical will have a light/dark mode the icons' colour will have to adapt..

I think it'll be easier to normalize integration icons with : https://github.com/simple-icons/simple-icons as it provides almost all the platforms infisical supports. IMO this will provide an unified design across icons, and will be easier to implement new ones, no need to search for the perfect one online, just import it from the library or through their CDN and you're set!

And if the icon doesn't exist, no problem it's open source, we can add it ourselves to the library ✌️

Examples (through the CDN, with dark mode support):

URL pattern

  • <img height="32" width="32" src="https://cdn.simpleicons.org/[ICON SLUG]/[COLOR]/[DARK_MODE_COLOR]" />
  • <img height="18" width="18" src="https://cdn.simpleicons.org/github/black/white" />
  • <img height="18" width="18" src="https://cdn.simpleicons.org/supabase/black/white" />
  • ...

Results

You can give a look at their frontend if you want: https://simpleicons.org/

What do you think?

@Salman2301
Copy link
Contributor Author

@akhilmhdh or @maidul98 I want to work on integration logs after this PR... if it's get merged. I would like if anyone take a look.

@Salman2301
Copy link
Contributor Author

@Grraahaam Will take a look but it's probably out of scope for this PR. If you can let the team add it somewhere in the project milestone backlog for the light mode. I think that will be the best course of action for now.

@Grraahaam
Copy link
Contributor

It's probably out of scope for this PR

Maybe you're right, since it might impact more than just the integration page. 👍🏽

@akhilmhdh @vmatsiiako @dangtony98 What do you think about the above suggestion ?

@akhilmhdh
Copy link
Member

@Salman2301 can you kindly remove the images on left side. It doen't go with the ui at all. Kinda looks bad or mismatching

Copy link
Member

@akhilmhdh akhilmhdh left a comment

Choose a reason for hiding this comment

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

One more change would be top bar. The select is kinda weird or rather doesn't go with the ui

I would recommend keep the filter and sort as icon buttons to top right. Then when clicked they could select the various options as needed.

You can keep the filter like Radix Menu Edit > Menu

backend/src/services/integration/integration-dal.ts Outdated Show resolved Hide resolved
@Salman2301
Copy link
Contributor Author

@akhilmhdh Do you want the filter and sort to be one icon?

Icon click ⌄
 > Filter
     > Development
         > dev
     > Integration
        > ...
 > Sort
     > Sort options 1?

Or two icons one for filter one for sort?

@Salman2301
Copy link
Contributor Author

I use the same ui as the audit log page...

@LitoMore
Copy link

Hi, I'm the maintainer of the Simple Icons project.

Please note that the dark mode feature https://cdn.simpleiocns.org/github/black/white won't work in Safari.

See this: https://stackoverflow.com/questions/58493483/issue-with-embedded-svg-images-in-dark-mode

The <object> tag may have some security issues, so the better solution would be the <picture> tag. Here is an example:

<picture>
  <source media="(prefers-color-scheme: dark)" srcset="https://cdn.simpleicons.org/github/white">
  <source media="(prefers-color-scheme: light)" srcset="https://cdn.simpleicons.org/github/black">
  <img alt="GitHub" src="https://cdn.simpleicons.org/github/black">
</picture>

In the meantime, other CDN services exist, such as jsDelivr and unpkg. Here are their differences:

cdn.simpleicons.org

It supports brand colors and SVG inline color schemes for dark mode. This service is hosted on Vercel. It does not support history versions. It always provides the latest version of Simple Icons. Which means an icon is subject to removal. See removal of brands.

jsDelivr & unpkg

It's more professional for CDN, and it has a faster response speed. It supports history versions. But it does not have a fill color. The color is up to your HTML context.

Note: jsDelivr has Access-Control-Allow-Origin: * support, but unpkg does not. This means you cannot use the unpkg resource in CSS background: url(...).

Which solution would be better?

It depends on what you need about the image/icon.

If you only need black and white

The jsDelivr would be the best solution. You can use CSS invert() to get black and white from a single resource.

If you want the brand color

jsDelivr

You could bundle the simple-icons.json file only. It has the full icon list with its slug and brand color value. And use CSS mask to apply the brand color to your jsDelivr resource. Here is an example: https://github.com/LitoMore/simple-icons-figma/blob/a5c2fae7ddcb68985c54d7ac7a0f3a9c75cca34f/source/components/icon.tsx#L62-L68.

cdn.simpleicons.org

This is convenient but has a slower response than jsDelivr. And it has some limitations:

Consuming the Vercel resource of LitoMore/simple-icons-cdn rapidly may quickly reach the rate limit. We're using the Vercel free plan, so this solution is not friendly to some services with a large request volume.


Feel free to ping me if you have any questions.

@Grraahaam
Copy link
Contributor

Thanks @LitoMore for the usage clarification and suggestions! Great tool by the way 🚀

@maidul98
Copy link
Collaborator

maidul98 commented May 7, 2024

I think this is useful feature as users sometimes have 100s of integrations. One feedback, i think the two filters being visible from the start makes it look like a lot is going on. It would be best if the user can click some filter icon and then they see those options. By default the filters should also be set to defaults such as all integrations and all environments. After this, i think we can merge this in. Does this make sense?
Screenshot 2024-05-07 at 12 27 51 PM

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.

None yet

5 participants