UI: Update every icon for v7 design #18809

MichaelArestad · 2022-07-28T16:00:00Z

What I did

I updated every icon in Storybook. I also reworked the icon component a bit.

To do: Update the available icon list in the README

How to test

Is this testable with Jest or Chromatic screenshots?
Does this need a new example in the kitchen sink apps?
Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

code/lib/components/src/icon/icon.tsx

MichaelArestad · 2022-07-28T16:06:58Z

code/lib/components/src/icon/icon.tsx

+  );
+};
+
+export type IconType = keyof typeof icons;

 export interface SymbolsProps extends ComponentProps<typeof Svg> {
  icons?: IconKey[];
 }

 export const Symbols = memo<SymbolsProps>(({ icons: keys = Object.keys(icons) }) => (


This component differed from the one in the Design System in that there was an effort to make a symbol instead of using SVGs directly (sort of like a sprite). I left that functionality in tact here, but I am unsure if it is necessary. Thoughts?

code/lib/components/src/icon/icon.tsx

code/lib/components/src/icon/icon.stories.tsx

…size, and resolved ts error

…untForm

…esign System

docs/faq.md

kylegach

The code and visual output here looks great!

I took this PR on thinking that I could address the remaining checks, but ran into hiccups with both:

Failing unit test check
- There are two storyshots-related snapshots affected by these changes that need updated. I was unable to figure out how to update these locally, after running into issues trying to build addon-storyshots.
Chromatic UI tests
- There are a few failing stories that use place-hold.it for placeholder images, which has apparently become unreliable.
- I intended to replace these with a local utility to generate a placeholder image with specific dimensions, but:
  - I didn't see a good place for those to live within the examples directory
  - There are a lot more calls to that service than just those surfaced by this PR. Felt like it should be a separate effort.

yannbf · 2022-08-08T21:55:10Z

The code and visual output here looks great!

I took this PR on thinking that I could address the remaining checks, but ran into hiccups with both:

Failing unit test check

There are two storyshots-related snapshots affected by these changes that need updated. I was unable to figure out how to update these locally, after running into issues trying to build addon-storyshots.

I'll take a look at this tomorrow!

Chromatic UI tests

There are a few failing stories that use place-hold.it for placeholder images, which has apparently become unreliable.

I intended to replace these with a local utility to generate a placeholder image with specific dimensions, but:

I didn't see a good place for those to live within the examples directory

There are a lot more calls to that service than just those surfaced by this PR. Felt like it should be a separate effort.

Fortunately this has been resolved already, I just merged a PR that addressed this issue 🎉

yannbf · 2022-08-12T08:34:50Z

I'll start adding my findings as comments, hope you don't mind!

I wonder why Chromatic didn't pick this up, I guess we don't have a story for such component.. but something went wrong with the spacing of the share button in docs mode:

And this is from an example using

yarn sandbox --template cra/default-js

code/lib/components/src/icon/icon.tsx

docs/faq.md

yannbf · 2022-08-12T09:22:11Z

I wonder if the icons becoming smaller is also intended

[previous] --------------------------------------------------- [this PR]

- The snapshot styles changed because the Button component was modified

code/lib/components/src/Button/Button.tsx

This reverts commit f72bbaa.

This reverts commit 3de325f.

This reverts commit 1320ba9.

domyen · 2022-08-13T02:37:50Z

Thanks Yann!

Re: icons getting smaller, @MichaelArestad I wonder if we should embiggen the docs icon so that the doc extends to the top and bottom of the area (similar to the profile icon)

re: Docs mode share button
I'm not seeing that on local in Safari or Chrome. Could it be something with the build process?

MichaelArestad · 2022-08-16T14:27:37Z

I wonder why Chromatic didn't pick this up, I guess we don't have a story for such component.. but something went wrong with the spacing of the share button in docs mode:

I was able to confirm this but only with yarn sandbox --template cra/default-js. @domyen may be right in that this feels like a build issue. This styling is missing:

This is also a bug in Next so it is unrelated to this PR. You can also see on the official storybook this styling is repeated a few times, which also isn't great. @thafryer can I get some help on this?

MichaelArestad · 2022-08-16T14:30:33Z

@domyen I tried making the docs icon larger and, after seeing how it looks, recommend sticking with the current version. Alternatively, we could make the component icon smaller (fourth option):

domyen · 2022-08-16T19:11:37Z

LGTM @MichaelArestad cc @yannbf

We resized the icons – thanks for the call out
We're punting on the weird share button in the built docs (that's unrelated to this PR)
@MichaelArestad can you follow up with CSF folks about what we need to add to migration.md

docs/faq.md

MichaelArestad added ui maintenance User-facing maintenance tasks labels Jul 28, 2022

MichaelArestad self-assigned this Jul 28, 2022

MichaelArestad commented Jul 28, 2022

View reviewed changes

code/lib/components/src/icon/icon.tsx Outdated Show resolved Hide resolved

MichaelArestad commented Jul 28, 2022

View reviewed changes

kylegach reviewed Jul 29, 2022

View reviewed changes

code/lib/components/src/icon/icon.tsx Outdated Show resolved Hide resolved

kylegach reviewed Jul 29, 2022

View reviewed changes

code/lib/components/src/icon/icon.stories.tsx Outdated Show resolved Hide resolved

MichaelArestad added 3 commits August 2, 2022 10:56

Initial icon update

36ad747

Tweak type requirement

1bf946a

Updated Icons to be more explicit thanks to Jono

d33222e

MichaelArestad force-pushed the update-every-icon branch from c50eff0 to d33222e Compare August 2, 2022 17:04

MichaelArestad added 8 commits August 3, 2022 14:32

Fix typescript errors - props Shaun on this one

a65d6a7

Removed unused bindin

d12dfc4

Fixed missing chevrons, fixed icon size in buttons, fixed alert icon …

cbe28f9

…size, and resolved ts error

Fixed button sizes and made slight tweak to alignment of icon in Acco…

c1b108e

…untForm

Made alert icon slightly smaller in release_notes.tsx

9694d53

Added Operating System icons and updated rule

4f2d6d4

Fix MenuButton size

c693d12

Remove icons list from docs FAQ

b482044

MichaelArestad marked this pull request as ready for review August 4, 2022 22:01

Update NotificationItem and updated a handful of icons to match the D…

9a42687

…esign System

MichaelArestad requested a review from kylegach August 8, 2022 14:24

kylegach reviewed Aug 8, 2022

View reviewed changes

docs/faq.md Show resolved Hide resolved

kylegach reviewed Aug 8, 2022

View reviewed changes

domyen and others added 2 commits August 11, 2022 18:31

Merge branch 'next' into pr/18809

59f9f49

Merge branch 'next' into update-every-icon

3f1f574

yannbf reviewed Aug 12, 2022

View reviewed changes

code/lib/components/src/icon/icon.tsx Show resolved Hide resolved

yannbf reviewed Aug 12, 2022

View reviewed changes

docs/faq.md Show resolved Hide resolved

update stories and snapshots

6d1585a

- The snapshot styles changed because the Button component was modified

yannbf reviewed Aug 12, 2022

View reviewed changes

code/lib/components/src/Button/Button.tsx Show resolved Hide resolved

domyen added 6 commits August 12, 2022 17:51

Embiggen the sidebar icons by 2px

f72bbaa

Viewport copyedit from "rotate" to "swap" dimensions

3de325f

Use the appropriate share icon in Docs Preview Toolbar

1320ba9

Revert "Embiggen the sidebar icons by 2px"

2237d97

This reverts commit f72bbaa.

Revert "Viewport copyedit from "rotate" to "swap" dimensions"

24da49e

This reverts commit 3de325f.

Revert "Use the appropriate share icon in Docs Preview Toolbar"

f65602f

This reverts commit 1320ba9.

Add icon embed to FAQ

bb3a2d8

Updated bookmark, bookmarkhollow, document, and folder icons

f1fb9af

shilman requested changes Aug 17, 2022

View reviewed changes

docs/faq.md Outdated Show resolved Hide resolved

shilman added 2 commits August 17, 2022 08:36

Review feedback

162463e

Merge branch 'next' into update-every-icon

820b8e4

shilman changed the title ~~Update every icon~~ UI: Update every icon for v7 design Aug 17, 2022

shilman merged commit 77dbaab into next Aug 17, 2022

shilman deleted the update-every-icon branch August 17, 2022 01:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Update every icon for v7 design #18809

UI: Update every icon for v7 design #18809

MichaelArestad commented Jul 28, 2022 •

edited

Loading

MichaelArestad Jul 28, 2022

kylegach left a comment

yannbf commented Aug 8, 2022

yannbf commented Aug 12, 2022 •

edited

Loading

yannbf commented Aug 12, 2022

domyen commented Aug 13, 2022

MichaelArestad commented Aug 16, 2022 •

edited

Loading

MichaelArestad commented Aug 16, 2022

domyen commented Aug 16, 2022

UI: Update every icon for v7 design #18809

UI: Update every icon for v7 design #18809

Conversation

MichaelArestad commented Jul 28, 2022 • edited Loading

What I did

How to test

MichaelArestad Jul 28, 2022

Choose a reason for hiding this comment

kylegach left a comment

Choose a reason for hiding this comment

yannbf commented Aug 8, 2022

yannbf commented Aug 12, 2022 • edited Loading

yannbf commented Aug 12, 2022

domyen commented Aug 13, 2022

MichaelArestad commented Aug 16, 2022 • edited Loading

MichaelArestad commented Aug 16, 2022

domyen commented Aug 16, 2022

MichaelArestad commented Jul 28, 2022 •

edited

Loading

yannbf commented Aug 12, 2022 •

edited

Loading

MichaelArestad commented Aug 16, 2022 •

edited

Loading