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

Add TypeScript definitions for EuiConfirmModal #1260

Merged
merged 3 commits into from
Oct 24, 2018

Conversation

pugnascotia
Copy link
Contributor

@pugnascotia pugnascotia commented Oct 24, 2018

Add TypeScript definitions for EuiConfirmModal, plus other definition fixes.

Notes:

  • I changed the AnyProps definition to map to any values, because I observed that the existing definition was too restrictive.
  • However it then removed it entirely from EuiTableRow because AnyProps wasn't necessary to allow e.g. key or data-foo props, and other table rows attributes are deprecated in HTML5.
  • The color prop for EuiIcon actually allows 6 or 3 digit hex color codes, so I allowed string as a possible type. I left the existing values as documentation.

@pugnascotia pugnascotia force-pushed the revenge-of-the-ts-defs branch from d66b0d1 to 01109f1 Compare October 24, 2018 15:41
@pugnascotia pugnascotia changed the title Add TypeScript definitions for EuiConfirmModal and EuiFieldPassword Add TypeScript definitions for EuiConfirmModal Oct 24, 2018
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Looks great, only one change requested

src/components/table/index.d.ts Outdated Show resolved Hide resolved
@pugnascotia
Copy link
Contributor Author

@chandlerprall all updated

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM!

@pugnascotia pugnascotia merged commit d1268ea into elastic:master Oct 24, 2018
@pugnascotia pugnascotia deleted the revenge-of-the-ts-defs branch October 24, 2018 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants