Skip to content

Create ComponentChecklist component #1626

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

Merged
merged 6 commits into from
Nov 19, 2021
Merged

Create ComponentChecklist component #1626

merged 6 commits into from
Nov 19, 2021

Conversation

colebemis
Copy link
Contributor

@colebemis colebemis commented Nov 19, 2021

Adds a ComponentChecklist component in the /docs directory that will enable us to document the maturity of our components.

## Component checklist

<ComponentChecklist
  items={{
    propsDocumented: true,
    noUnnecessaryDeps: true,
    adaptsToThemes: true,
    adaptsToScreenSizes: true,
    fullTestCoverage: true,
    usedInProduction: true,
    usageExamplesDocumented: true,
    designReviewed: null,
    a11yReviewed: null,
    stableApi: false,
    addressedApiFeedback: false,
    hasDesignGuidelines: null,
    hasFigmaComponent: null
  }}
/>

CleanShot 2021-11-18 at 17 16 07

Known issues

Here are list of known issues with ComponentChecklist that I think are out of scope for this PR:

  • The component is not written in TypeScript. We don't have Gatsby configured to work with TypeScript yet so I wrote the component in vanilla JavaScript. Updating our Gatsby configuration to work with TypeScript felt out of scope for this PR.
  • The ComponentChecklist component needs to be imported in every markdown file that uses it. I plan to make a change in Doctocat that will allow us to make ComponentChecklist globally available in all markdown files. Until I make that change, ComponentChecklist will need to be explicitly imported.
  • ComponentChecklist items are hard to guess. You'll need to look at the definition of ComponentChecklist to know which checklist items to pass. I imagine most people will use ComponentChecklist by copying and pasting from other documentation pages, so I don't think this is a dealbreaker
  • You need to add the checklist heading in markdown. The heading (i.e. ## Component checklist) needs to be written directly in the .md or .mdx file. The heading can't be included in the ComponentChecklist component because it won't be listed in the table of contents.

Open questions

  • What should the heading of this checklist be? I'm using Component checklist for now but I'm open to other ideas.
  • Is the n/a state for a checklist item visually clear? I'm using the same icon that we use to represent a "skipped" GitHub action.

Next steps

  • Render this checklist on every component documentation page

Notes

This is a departure from my original implementation plan for component checklists. I wrote about why changed my implementation plan here.

@changeset-bot
Copy link

changeset-bot bot commented Nov 19, 2021

⚠️ No Changeset found

Latest commit: 40d955c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 19, 2021

size-limit report 📦

Path Size
dist/browser.esm.js 56.31 KB (0%)
dist/browser.umd.js 56.7 KB (0%)

@colebemis colebemis added the skip changeset This change does not need a changelog label Nov 19, 2021
@colebemis colebemis marked this pull request as ready for review November 19, 2021 01:35
@colebemis colebemis requested review from a team, siddharthkp and jfuchs November 19, 2021 01:35
Copy link
Member

@siddharthkp siddharthkp left a comment

Choose a reason for hiding this comment

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

This looks good! No changes needed here, suggested changes in doctocat.

@colebemis
Copy link
Contributor Author

@siddharthkp @jfuchs Is this n/a state more clear?

CleanShot 2021-11-19 at 08 46 15

@siddharthkp
Copy link
Member

siddharthkp commented Nov 19, 2021

Is this n/a state more clear?

It's very clear.

Bonus: now we make it pretty too 😋 I like the skip icon you had from octions! How do you feel about adding N/A to the text part?

@colebemis
Copy link
Contributor Author

Better?
CleanShot 2021-11-19 at 08 59 30

@siddharthkp
Copy link
Member

Love it!

Copy link
Contributor

@rezrah rezrah left a comment

Choose a reason for hiding this comment

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

Very cool @colebemis. As a name suggestion, how about "Component status", as provides more detailed explanation - and is directly related to - the status at the top of the page? Edit: NVM, you've updated it to this already, didn't see your updated screenshot 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
react skip changeset This change does not need a changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants