storybookjs / design-system

🗃 Storybook Design System
https://master--5ccbc373887ca40020446347.chromatic.com/
1.91k stars 585 forks source link

Pulled in `AuthorList` component from `frontpage` #321

Closed smithambera closed 2 years ago

smithambera commented 2 years ago

Fixes https://linear.app/chromaui/issue/CH-710/pull-author-and-authorlist-from-frontpage-into-ds

Description

Pulled the AuthorList component from https://github.com/storybookjs/frontpage into the DS in preparation for future work.

QA instructions

Confirm the styles (mostly) match [the designs]((https://www.figma.com/file/gYdEmX9zYowUR3K2SOua5J/Component-catalog?node-id=148%3A9975).

📦 Published PR as canary version: 7.0.4-canary.321.a50c445.0
:sparkles: Test out this PR locally via: ```bash npm install @storybook/design-system@7.0.4-canary.321.a50c445.0 # or yarn add @storybook/design-system@7.0.4-canary.321.a50c445.0 ```
domyen commented 2 years ago

Thanks Amber!

There are a few cases to refine that weren't covered in the first buildout. We should style and make stories for them while we're working on this component.

  1. Show more interactive
  2. Show more non-interactive (can happen when we're querying a 3rd party API like NPM). There's an existing story for this.

Figma file here

image

smithambera commented 2 years ago

Hey @domyen - can you send me that illustration? I didn't see it in frontpage or chromatic. I did find avatar.png in chromatic, but it's not quite the same:

Screen Shot 2021-11-20 at 12 34 29 PM

Did you want ^ that illustration ^ when there is only one author hidden and the one from the designs above if there is more than one author hidden? Also, it'd probably be preferable to get them as svgs!

domyen commented 2 years ago

Thanks for the callout Amber and amazing idea to have 1 user vs 2 users depending on how many are left to reveal. Yes, let's do what you suggested!

These images are in the <Icon> component as user and users respectively.

smithambera commented 2 years ago

Hey @domyen - it doesn't look like the current component supports interactivity for the hidden authors. What is your desired behavior there?

Additionally, based on the way the component is presently built, there would be no way to see the non-interactive state as we're expecting this component to get the entire list of authors from the get-go.

domyen commented 2 years ago

it doesn't look like the current component supports interactivity for the hidden authors. What is your desired behavior there?

If we can get the full list of people, the intended behavior is when you click on the "+# more" that the full list of people is rendered in place of the "+# more".

Additionally, based on the way the component is presently built, there would be no way to see the non-interactive state as we're expecting this component to get the entire list of authors from the get-go.

Some services like NPM do not give us the full list of people afaict. Check out what's happening in the "Made by" section of this addon page.

smithambera commented 2 years ago

Some services like NPM do not give us the full list of people afaict. Check out what's happening in the "Made by" section of this addon page.

The existing AuthorList code is expecting all of the authors and only showing 5:

const authorsSubset = useMemo(() => authors.slice(0, 5), [authors]);
const more = useMemo(() => authors.length - 5, [authors]);

In that link you shared, the component has access to all of the authors:

Screen Shot 2021-12-01 at 2 05 09 PM
domyen commented 2 years ago

Doh! Thanks for clearing that up Amber – let's remove the non-interactive state.