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

Building icons from Figma #709

Merged
merged 6 commits into from
Nov 16, 2021
Merged

Building icons from Figma #709

merged 6 commits into from
Nov 16, 2021

Conversation

dbanksdesign
Copy link
Contributor

@dbanksdesign dbanksdesign commented Nov 15, 2021

Issue #, if available:

Description of changes: To get ready for launch and to make sure our icons match what is in Figma exactly, I pulled the SVG source from Figma into our own SVG files. Doing so brings our number of icons from ~1800 to ~1500.

As discussed, in the future we will probably remove all these icons down the road, but at least at this point we can be sure the icon name and design match what is shown in Figma.

  • Updated icon primitive docs
  • Added icon SVGs in the ui package and generated the JSX code from those

Because Github chokes with this huge PR, here are the relevant files:

CleanShot.2021-11-15.at.13.55.11.mp4

TODO

  • Update Icons docs

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@changeset-bot
Copy link

changeset-bot bot commented Nov 15, 2021

🦋 Changeset detected

Latest commit: 3a6d559

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@aws-amplify/ui Patch
@aws-amplify/ui-react Patch
@aws-amplify/ui-vue Patch
@aws-amplify/ui-angular Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ericclemmons
Copy link
Contributor

This removes all the icons except ~30 for our own use-cases in other primitives and components.

GitHub is struggling to let me comment on the files tab, so I'll add this here.

The only question I have about icons is whether the ones we need become a public API or not.

In the interest of reducing our exported API, we have a couple options here:

  1. Keep the images as SVGs and import them as a data URL for use in <Icon> or <Image>:

    https://esbuild.github.io/content-types/#data-url

  2. Transform/build SVGs into React components (e.g. https://github.com/gregberge/svgr), but only reference with relative paths vs. being a named export.

  3. As you or @hvergara has suggested before, have a separate @aws-amplify/ui-icons repo. But, we have this listed as a devDependencies so that it's bundled vs. being a published, external dependency:

    https://tsup.egoist.sh/#excluding-packages

@dbanksdesign
Copy link
Contributor Author

@ericclemmons I updated the description to include links to relevant files on the branch. I actually started to use svgr, but it didn't do quite what I wanted like using some of our internal files and other dependencies so it would work like our generic Icon component. Here is what a icon react component looks like:

https://github.com/aws-amplify/amplify-ui/blob/icon-update/packages/react/src/primitives/Icon/icons/IconAdd.tsx

import classNames from 'classnames';

import { ComponentClassNames } from '../../shared';
import { View } from '../../View';

export const IconAdd = (props) => {
  const { className, ...rest } = props;
  return (
    <View
      as="span"
      width="1em"
      height="1em"
      className={classNames(ComponentClassNames.Icon, className)}
      {...rest}
    >
      <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
        <path d="M19 13h-6v6h-2v-6H5v-2h6V5h2v6h6v2z" fill="currentColor" />
      </svg>
    </View>
  );
};

We could remove these icons from the exports, but then we wouldn't be able to have any pre-built icons in our Figma file and wouldn't be able to use the icons in our docs site. I think ~30 exported icons would not be too big of a deal and would give customers at least something to use to get started. But I'm open to removing them as exports if everyone agrees.

@dbanksdesign dbanksdesign changed the title Removing all material icons for a subset Building icons from Figma Nov 15, 2021
@dbanksdesign
Copy link
Contributor Author

Updated the PR to add the icons from the Figma file in an icons folder in the UI package to act as the source of our icons.

@dbanksdesign dbanksdesign temporarily deployed to ci November 15, 2021 22:55 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 15, 2021 22:55 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 15, 2021 22:55 Inactive
@dbanksdesign dbanksdesign marked this pull request as ready for review November 15, 2021 23:27
Co-authored-by: Hector Vergara <hvergara@users.noreply.github.com>
@dbanksdesign dbanksdesign temporarily deployed to ci November 16, 2021 18:46 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 16, 2021 18:46 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 16, 2021 18:46 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 16, 2021 18:48 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 16, 2021 18:48 Inactive
@dbanksdesign dbanksdesign temporarily deployed to ci November 16, 2021 18:48 Inactive
@dbanksdesign dbanksdesign merged commit 3cc1c15 into main Nov 16, 2021
@dbanksdesign dbanksdesign deleted the icon-update branch November 16, 2021 19:18
@github-actions github-actions bot mentioned this pull request Nov 16, 2021
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.

3 participants