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

Create Icon component #20133

Closed
6 tasks done
timroes opened this issue Dec 6, 2022 · 16 comments
Closed
6 tasks done

Create Icon component #20133

timroes opened this issue Dec 6, 2022 · 16 comments
Assignees
Labels

Comments

@timroes
Copy link
Contributor

timroes commented Dec 6, 2022

We would like to use more custom SVG icons over just using Font Awesome. We should create a custom Icon component that aligns handling those icons. At the moment we're just exporting custom React components for each of them, which all come with a bit different behavior (regarding size, how we handle colors, etc.).

I think the new icon component should do the following:

  • Allow specifying the icon type as a string, so it's easier usable, e.g. <Icon type="caret" />
  • Should allow specifying size of the icon in shirt sizes, e.g. <Icon size="md" /> to match our regular icon sizes we have (and align them.
  • Allow specifying color for the icon from a predefined set of colors we usually use or text for using currentColor.
  • Allow specifying a className as we should for all base UI components.

That could allow us to manage the actual icons even in pure svg files if we want. We could consider loading all icons in one svg once only on the page and referencing them via symbol and use so we don't need to duplicate the icon SVG if we have an icon used multiple times on the same page.

Tasks

Preview Give feedback
  1. area/frontend
    krishnaglick
  2. area/frontend platform-move/requires-grooming team/platform-move
    nora-airbyte
  3. area/frontend platform-move/requires-grooming team/platform-move
    chandlerprall
  4. area/frontend platform-move/requires-grooming team/platform-move
    chandlerprall
@octavia-squidington-iv
Copy link

cc @airbytehq/frontend

@edmundito
Copy link
Contributor

@krishnaglick Reassigning this to you!

@edmundito edmundito assigned krishnaglick and unassigned matter-q Feb 8, 2023
@krishnaglick
Copy link
Contributor

@timroes What do you mean about color. Are just generic css color's fine?

@timroes
Copy link
Contributor Author

timroes commented Feb 14, 2023

@krishnaglick I think we want to have a limited set of colors only available, like "primary", "subdued", or such. We'd need to actually check what colors we're at the moment using in our UI, and what colors we can break that down to. Might be worth syncing with Nico about which colors we'll need exactly.

@Upmitt
Copy link
Contributor

Upmitt commented Feb 20, 2023

@krishnaglick @timroes - I did this list of colors used for icons https://www.figma.com/file/gMNRYVJav25pdsoxAyW7Qp/00_01_WEB-APP-LIBRARIES?node-id=2514%3A4578&t=2dhs2PhfGlCBSMY6-4 - do you want to plan a meeting to talk about it?

@timroes
Copy link
Contributor Author

timroes commented Feb 20, 2023

@krishnaglick @timroes - I did this list of colors used for icons https://www.figma.com/file/gMNRYVJav25pdsoxAyW7Qp/00_01_WEB-APP-LIBRARIES?node-id=2514%3A4578&t=2dhs2PhfGlCBSMY6-4 - do you want to plan a meeting to talk about it?

Thanks for the list.

Is there a chance that we can narrow down the amount of grey shades a bit for colors we want to use in the future? It looks like we might not need that amount of slightly different shaded greys?

@Upmitt
Copy link
Contributor

Upmitt commented Feb 20, 2023

@timroes yes sure! I'll do that, just need to check different screens before reducing this list

@flash1293
Copy link
Contributor

These icons are used in designs by @Upmitt , they should be available in the webapp as well: https://www.figma.com/file/gMNRYVJav25pdsoxAyW7Qp/00_01_WEB-APP-LIBRARIES?node-id=2%3A394&t=p4l9lbH6hV6iquOX-0

@krishnaglick
Copy link
Contributor

krishnaglick commented Feb 22, 2023

My current color shortlist is "primary" | "darkBlue" | "action" | "grey" | "success" | "error" | "warning"
I think we could remove darkBlue. Maybe rename grey to disabled?
Text uses darkBlue and grey I believe, so maybe they're worth keeping. The colors are typed so I'm not super concerned.

@josephkmh
Copy link
Contributor

"primary" | "darkBlue" | "action" | "grey" | "success" | "error" | "warning"

Can I suggest that we only use semantic names, not names of colors ("darkBlue" | "grey")? I guess that's what you're getting at @krishnaglick.

IMO "action" is also kind of ambiguous. I would almost prefer the "primary" | "secondary" | "tertiary" pattern there instead if we can't come up with a different meaningful name.

@krishnaglick
Copy link
Contributor

Bump @Upmitt This is lower priority than what you're busy with but just want to keep this on your radar!

@josephkmh
Copy link
Contributor

@krishnaglick any update on this component? Would be great to get it built asap, because it is a building block for some other UI library components.

@edmundito
Copy link
Contributor

Spoke with @chandlerprall, he will take over this

@josephkmh
Copy link
Contributor

@chandlerprall are we able to close this issue now? Looking at https://github.com/airbytehq/airbyte-platform-internal/pull/7709/files

@chandlerprall
Copy link
Contributor

This is also tracking other icon component issues - tasks in the description. I don't have a preference if we want to keep this around as a meta issue or close it. Feels more like Tim's brain child for icon asks, so maybe wait for him to get back and see what a preference may be?

@timroes
Copy link
Contributor Author

timroes commented Jan 26, 2024

Closing as we have a solid Icon component by now.

@timroes timroes closed this as completed Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests