fix(EuiBasicTable): fixed EuiBasicTable proptypes of itemId

[pranshuchittora]

Summary

Closes #3130
Summarize your PR. If it includes design elements include a screenshot or gif.
As of now, I have added a type the propTypes of ItemId (number).
Another approach is that I can convert the given input to String.

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

[kibanamachine]

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?

[cchaos]

Jenkins test this

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_3133/

[cchaos]

@pranshuchittora Can you describe how you found the issue to be that number was not supported?

[chandlerprall]

The root case here is the re-use of ItemId<T> type for both EuiBasicTable's itemId and CollapsedItemActions's itemId. This was an oversight during these components' translation to TypeScript.

EuiBasicTable uses its itemId to know what field on an object is its ID, thus the string | ((item: T) => string) definition. However, CollapsedItemActions's itemId is the resolved value (either a string or a number are valid, thus the change).

The proper way to fix this is to re-define CollapsedItemAction's itemId as string | number, not ItemId<T>. expanded_item_actions.tsx should be updated similarly.

[pranshuchittora]

Why the ItemId has a generic type?
Why it can't just be a string or a number?

eui/src/components/basic_table/table_types.ts

Line 8 in f8bb840

export type ItemId<T> = string | ((item: T) => string);

[chandlerprall]

It needs the generic type because, when it is a function, it is passed the item as its argument. For the tables, the item type is represented by the generic. The code using itemId as a function is at

eui/src/components/basic_table/basic_table.tsx

Line 96 in fa2ec10

export function getItemId<T>(item: T, itemId?: ItemId<T>) {

[chandlerprall]

Looks good, but needs to make the same changes to src/components/basic_table/expanded_item_actions.tsx

[pranshuchittora]

@chandlerprall please review the new changes :)

[chandlerprall]

Changes LGTM; Pulled & tested locally. I also merged in master to correct the changelog entry placement (releases often screw up open PR changelogs)

[chandlerprall]

Thanks for the bugfix @pranshuchittora !