Skip to content

converts EuiForm (and family) to TypeScript#2896

Merged
chandlerprall merged 14 commits intoelastic:masterfrom
dimitropoulos:form-ts
Feb 26, 2020
Merged

converts EuiForm (and family) to TypeScript#2896
chandlerprall merged 14 commits intoelastic:masterfrom
dimitropoulos:form-ts

Conversation

@dimitropoulos
Copy link
Contributor

closes: #2892

@kibanamachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@snide
Copy link
Contributor

snide commented Feb 22, 2020

Thanks again for the PR. We'll mostly be silent over the weekend, but will give this a review on Monday.

@chandlerprall chandlerprall self-assigned this Feb 24, 2020
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.

Thanks for another detailed & thorough conversion!

src/components/form/form.test.tsz was renamed with an invalid file extension

Reviewed most of the changes; left the form vs. div prop stuff until that conversation & decision happens.

Have not thoroughly reviewed changes to exports, as there is a request to decrease the scope of some of those.

@dimitropoulos
Copy link
Contributor Author

fixed form.test.tsz -> form.test.tsx in ffc0e27 (whoops! 😅 I was moving very fast the night I did these.. heh.)

Otherwise, I think I've addressed all other feedback and just need some direction on what/how you want to be exported. I know with this kind of thing (since I've now exported everything it'll be literally just deleting single lines in the various index.ts in subdirectories like field_number/) it can be easier to just make the change rather than writing up detailed descriptions of the changes in a PR. I'm happy to do anything that works well for you all - so feel free to do what's easiest for you.

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.

Everything's looking great! Please reset those extra exports and I'll do one last check of the build/eui.d.ts.

@dimitropoulos
Copy link
Contributor Author

@chandlerprall see #2896 (comment) and #2896 (comment), but otherwise I think we're all set here 😃!

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.

Changes LGTM; pulled & tested locally, including impact to eui.d.ts which removes some previously exported values that were never exported on the JS side (bugfix).

@chandlerprall chandlerprall merged commit 15aaf89 into elastic:master Feb 26, 2020
@dimitropoulos dimitropoulos deleted the form-ts branch February 27, 2020 01:11
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.

EuiForm needs to be converted to TypeScript

5 participants