Translate EuiTabs to TS

[junlarsen]

Summary

Closes #2645

Converted EuiTabs to TypeScript

Checklist

• Check against all themes for compatability 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

[elasticmachine]

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?

[chandlerprall]

Thank you for this PR!

A few requests, mostly around the TypeScript patterns we've established in EUI. In addition to the comments in this review:

src/components/tabs/tabbed_content/index.ts

• Include EuiTabbedContentProps and EuiTabbedContentTabDescriptor in the export (so they can be re-used by other projects)

src/components/tabs/index.ts

• Export EuiTabsProps from './tabs'
• Export EuiTabbedContentProps from './tabbed_content'

src/components/tabs/index.d.ts

• Delete this file

src/components/index.d.ts

• Remove /// <reference path="./tabs/index.d.ts" />

---

When this PR is stable I'll also do a custom EUI build with these types to verify usages in Kibana.

[chandlerprall]

Couple more requests, then I'll drop these into Kibana for another round of testing.

Additionally, there's some eslint errors which can be automatically fixed by running yarn lint-es-fix and committing the results.

[chandlerprall]

Built EUI and tested in Kibana, found these three things -

EuiTabbedContentTab export

(see inline comment associated with this review)

Snapshots

Two jest snapshots need to be updated, please run yarn test-unit -u and commit the results.

Sinon

tabbed_content.test.tsx uses sinon for one test, and we haven't installed @types/sinon as we want to move away from that library and rely solely on Jest. Please remove the sinon import and lines 39-47 should be changed to

      test('is called when a tab is clicked', () => {
        const onTabClickHandler = jest.fn();
        const component = mount(
          <EuiTabbedContent onTabClick={onTabClickHandler} tabs={tabs} />
        );
        findTestSubject(component, 'kibanaTab').simulate('click');
        expect(onTabClickHandler).toBeCalledTimes(1);
        expect(onTabClickHandler).toBeCalledWith(kibanaTab);
      });

[chandlerprall]

Thanks for these changes! Now waiting for two things (not on you) -

• While testing, I hit a case where the as const syntax was complained about in the unit tests. I've had trouble reproducing, but did hit this on another branch. I'm going to upgrade our testing packages to make sure the error doesn't come back in the future.
• We have a Contributor License Agreement (CLA) that is required to be signed by a contributor before we can accept their pull request. I see in your github profile that you are a high school student, which may impact that process. I've requested input from our legal team and will let you know when there's an answer.

[chandlerprall]

Quick update on both points:

• babel package upgrade is underway (Allow as const syntax in tests #2733); tests now pass and code compiles, but scss is breaking at build
• conversation around the CLA has started but no answer(s) yet

[chandlerprall]

@supergrecko Good news! We can accept your signing as is! You can access the Contributor License Agreement at https://www.elastic.co/contributor-agreement - please use the same email address as your github account so our automation can verify.

I should have the babel package upgrade finished up in a couple days.

Thank you very much for your patience with me on this.

[chandlerprall]

jenkins test this

[chandlerprall]

Two lint errors:

src/components/tabs/tab.tsx
57:13 error Replace (rest·as·AnchorHTMLAttributes<HTMLAnchorElement>) with rest·as·AnchorHTMLAttributes<HTMLAnchorElement> prettier/prettier
70:11 error Replace (rest·as·ButtonHTMLAttributes<HTMLButtonElement>) with rest·as·ButtonHTMLAttributes<HTMLButtonElement> prettier/prettier

These are fixable with yarn lint-es --fix and then this should be good to merge.

[junlarsen]

That's strange, did we change any eslint/prettier rules? Those two sets of parentheses were added during an earlier ESLint run (see 49895d0)

When I run yarn lint-es --fix it's not being changed either. I rebased against master and reran but without any success. Let me know if you want me to manually change it.

[chandlerprall]

Hmm, that is odd. I went back to the 49895d0 commit where the parenthesis were introduced and it still complains about them. I'm not sure what would cause the difference for you.

Please go ahead and manually change those two lines.

[chandlerprall]

jenkins test this

[chandlerprall]

Changes LGTM; Pulled and tested locally.

[chandlerprall]

Thanks for the conversion, @supergrecko !