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

set intent types to string #1341

Merged
merged 2 commits into from Oct 25, 2021
Merged

set intent types to string #1341

merged 2 commits into from Oct 25, 2021

Conversation

zkuzmic
Copy link
Contributor

@zkuzmic zkuzmic commented Oct 6, 2021

Overview
There are a couple issues having to do with the intent prop on various components:

  1. The types (in index.d.ts and those defined via PropTypes) do not account for the fact that user-defined intents can be added via theming.
  2. Implementation of the intent prop is inconsistent from one component to the next. See the table below.
Component Supports user-defined intents? Supported default intents
Alert All (none, info, success, warning, danger)
Button success, danger
CornerDialog success, danger
Dialog success, danger
IconButton success, danger
InlineAlert All (none, info, success, warning, danger)
MenuItem ❌ (icon color, but not text) success, danger, info
StatusIndicator success, danger, info
TableRow none, success, danger, warning
Toast All (none, info, success, warning, danger)

In an ideal world, these components would be consistent and the types could be generated based on what user-defined intents there are, if any. Doing that seems worthwhile, but that's going to be a bigger initiative. I think this will solve the problem for now.

Resolves #1319

Screenshots (if applicable)

Documentation

  • Updated Typescript types and/or component PropTypes
  • Added / modified component docs
  • Added / modified Storybook stories

@netlify
Copy link

netlify bot commented Oct 6, 2021

✔️ Deploy Preview for evergreen-storybook ready!

🔨 Explore the source changes: 39984a8

🔍 Inspect the deploy log: https://app.netlify.com/sites/evergreen-storybook/deploys/6176f009158fca0007bf60db

😎 Browse the preview: https://deploy-preview-1341--evergreen-storybook.netlify.app

@@ -17,7 +17,7 @@ export type PositionTypes =
| 'bottom-right'
| 'left'
| 'right'
export type IntentTypes = 'none' | 'info' | 'success' | 'warning' | 'danger'
Copy link
Contributor

Choose a reason for hiding this comment

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

RIP

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.

Missing intent value 'info' in Menu.Item prop types
2 participants