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

docs: nest component recipes under components #1715

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Jul 31, 2023

Summary:

  • move component recipes (now referred to as implementation examples) under related components
  • label them as such in the storybook UI (using a custom badge)
  • note: some recipes are used in stories, which breaks convention
  • update snapshots for relevant decorator and component changes.
  • remove recipe and page references so new ones don't start appearing

Recipes used in other recipes and pages are pending removal. As a reminder to the bigger context... canonical recipes are those which are specific to a product and will live within the product's storybook/documentation. implementation examples are sandboxed code examples that are composed of a set of other EDS components which might describe a recommended layout. As examples, they are not exported, so we move them alongside other examples in storybook to reduce confusion.

Test Plan:

  • verify updated snapshots and chromatic diff.s

@booc0mtaco booc0mtaco requested a review from a team July 31, 2023 18:43
@github-actions
Copy link

github-actions bot commented Jul 31, 2023

size-limit report 📦

Path Size
components 95.49 KB (+0.01% 🔺)
styles 32.76 KB (0%)

@booc0mtaco booc0mtaco added the do not merge PRs that should not be merged (even if the build is green or there are approvals) label Jul 31, 2023
@booc0mtaco
Copy link
Contributor Author

Holding merge on this one until #1713 has merged (so we can remove the recipes used in the soon-to-be-deleted pages)

@jinlee93
Copy link
Contributor

jinlee93 commented Aug 2, 2023

Does this mean we won't have the recipes SB folder anymore? What if a new recipe involves more than one EDS component?

@booc0mtaco
Copy link
Contributor Author

Does this mean we won't have the recipes SB folder anymore? What if a new recipe involves more than one EDS component?

Correct. After looking thru things, having the recipes broken out as we were doing created two kinds of confusion:

  • Recipes are, by definition, compositions that are product specific. But stuff in storybook should be available to be imported?
  • We already had some stories that were combining multiple components, and based on feedback and questions (and the fact that the code examples show what's going on) the intent is clear. Why were some broken out separately?

For these implementation examples, there's usually a "main component" and subordinate components used to demonstrate how to use it. This gets us to a spot where we can be more clear on what the word "recipe" means outside of consuming projects. Happy to chat more about this, as it seemed MOSTLY clear to me as I sorted thru approaches!

Copy link
Contributor

@jinlee93 jinlee93 left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation!

@@ -59,3 +74,181 @@ export const Dragging: StoryObj<Args> = {
elevation: 'dragging',
},
};

export const ButtonActionCalloutCard: StoryObj<Args> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait what about putting the recipes under {component}/recipes/ via title: 'Components/Card/recipes or do you think the badge does the job?

Copy link
Contributor

Choose a reason for hiding this comment

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

could also consider always naming the stories "xx Example"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These could work, actually. I wanted to lean away from using that Recipe word in EDS, so we didn't create ambiguity on what a recipe is. but it may make sense to separate out these stories from the more normal stories.

Let's have a chat about that in our next sync :) I can create a ticket with some options to prep discussion.

Copy link
Contributor

@anniehu4 anniehu4 left a comment

Choose a reason for hiding this comment

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

love this

.storybook/preview.tsx Outdated Show resolved Hide resolved
@booc0mtaco
Copy link
Contributor Author

Gonna expand this one just a touch to include the now-unused recipes along with the generators for new recipes and pages to close the loop

@booc0mtaco booc0mtaco removed the do not merge PRs that should not be merged (even if the build is green or there are approvals) label Aug 2, 2023
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #1715 (d9fb2b4) into next (8e64702) will decrease coverage by 0.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             next    #1715      +/-   ##
==========================================
- Coverage   92.47%   92.28%   -0.20%     
==========================================
  Files         143      146       +3     
  Lines        2498     2579      +81     
  Branches      640      664      +24     
==========================================
+ Hits         2310     2380      +70     
+ Misses        187      183       -4     
- Partials        1       16      +15     
Files Changed Coverage Δ
...mponents/InlineNotification/InlineNotification.tsx 95.83% <ø> (ø)
src/components/Slider/Slider.tsx 88.46% <ø> (+1.92%) ⬆️
src/components/Table/Filters.tsx 79.62% <ø> (ø)
src/components/Table/StackedCardsToTable.tsx 96.15% <ø> (ø)

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

- move individual implementation examples under related components
- label them as such
- note: some recipes are used in stories, which breaks convention
- update snapshots
- remove plop integrations for creating new recipes and pages
- remove lingering recipes
@booc0mtaco booc0mtaco merged commit d8a4f6a into next Aug 2, 2023
6 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EDS-1085 branch August 2, 2023 22:09
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