Skip to content

Convert EuiDescribedFormGroup to typeScript#2810

Merged
elizabetdev merged 7 commits intoelastic:masterfrom
elizabetdev:described-form-group-ts
Feb 3, 2020
Merged

Convert EuiDescribedFormGroup to typeScript#2810
elizabetdev merged 7 commits intoelastic:masterfrom
elizabetdev:described-form-group-ts

Conversation

@elizabetdev
Copy link
Contributor

@elizabetdev elizabetdev commented Jan 30, 2020

Summary

Closes #2649.

This is basically an attempt to convert the EuiDescribedFormGroup to typescript.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation examples
  • Added or updated jest tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@elizabetdev elizabetdev changed the title Convert described form group to typeScript Convert EuiDescribedFormGroup to typeScript Jan 30, 2020
@elizabetdev elizabetdev mentioned this pull request Jan 30, 2020
9 tasks
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Nice job! A couple things below

Also, rename described_form_group/index.tsx to described_form_group/index.ts

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

These 3 all go together. Should be the last necessary changes.

elizabetdev and others added 3 commits January 31, 2020 19:16
Co-Authored-By: Greg Thompson <thompsongl@users.noreply.github.com>
Co-Authored-By: Greg Thompson <thompsongl@users.noreply.github.com>
Co-Authored-By: Greg Thompson <thompsongl@users.noreply.github.com>
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Almost! Caught something while checking in Kibana

Need to add the idAria prop to EuiDescribedFormGroupProps: idAria?: string

@elizabetdev
Copy link
Contributor Author

@thompsongl the idAria was deprecated on #2783.

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Ah I missed that. This is good to go, then

@elizabetdev elizabetdev merged commit a83c853 into elastic:master Feb 3, 2020
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.

EuiDescribedForm needs to be converted to TS

3 participants