Fixed description prop in EuiTable

[hetanthakkar]

Summary

Fixed issue #4333 : Description prop in EuiTable for column description and updated test snapshots.

For a brief overview of implementation details I have implemented this feature similar to DefaultItemAction as mentioned in this comment: #4333 (comment)

Checklist

• Check against all themes for compatibility in both light and dark modes
• Checked in mobile
• Checked in Chrome, Safari, Edge, and Firefox
• Props have proper autodocs and playground toggles
• Added documentation
• Checked Code Sandbox works for the any docs 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?

[elizabetdev]

Hi @hetanthakkar1,

Thanks for opening this PR. I tested and it works. 🎉

From a design perspective, we need a visual hint to show that the column headers contain a description. So we can take this example as a design solution: https://elastic.github.io/eui/#/tabular-content/tables#adding-sorting-to-a-table

As the above example shows, when a column header has a description we should include the following icon:

<EuiIcon
  size="s"
  color="subdued"
  type="questionInCircle"
/>

Then we should update all the examples that are using a tooltip like this one, to use a description prop rather than passing a tooltip.

So instead of adding a tooltip:

eui/src-docs/src/views/tables/sorting/sorting.js

Lines 95 to 115 in dbcf763

	{
	field: 'github',
	name: (
	<EuiToolTip content="Their mascot is the Octokitty">
	<span>
	Github{' '}
	<EuiIcon
	size="s"
	color="subdued"
	type="questionInCircle"
	className="eui-alignTop"
	/>
	</span>
	</EuiToolTip>
	),
	render: (username) => (
	<EuiLink href={`https://github.com/${username}`} target="_blank">
	{username}
	</EuiLink>
	),
	},

.

We should:

{
  field: 'github',
  name: 'Github',
  description: 'Their mascot is the Octokitty',
  render: (username) => (
    <EuiLink href={`https://github.com/${username}`} target="_blank">
    {username}
    </EuiLink>
  ),
},

[hetanthakkar]

@miukimiu What do you think

[thompsongl]

That result in display of both the title and description like this

Going back to the original issue:

A description of the column (will be presented as a title over the column header)

I read "presented as a title over the column header" to mean that we will use the native browser title element hover effect, not an EuiToolTip. The action item button case is different in that it's not in the header, and it does not describe a column (it describes the action list).

It makes more sense to me to simply do title={description || innerText} in the locations in question, and remove the new EuiToolTip elements.

[hetanthakkar]

@thompsongl You are correct! WIll make the required changes

[hetanthakkar]

@thompsongl Please review

[elizabetdev]

LGTM! 🎉

Tested locally in Chrome, Safari, Edge, and Firefox.

As @thompsongl mentioned

I read "presented as a title over the column header" to mean that we will use the native browser title element hover effect, not an EuiToolTip. The action item button case is different in that it's not in the header, and it does not describe a column (it describes the action list).

It makes sense to use the native browser title and it also makes sense to leave the "Adding sorting to a table" example with the tooltips. So in case, consumers need a more detailed description they can use that pattern. 🙂

Let's wait for @thompsongl approval and we're good to merge.

[hetanthakkar]

@miukimiu Can you please take a look at this too: #4753 (comment)

[hetanthakkar]

@thompsongl @myasonik Can you please review!? I think I have made the necessary changes but feel free to correct me

[myasonik]

@hetanthakkar1 Thanks for the quick turnaround! It's on the right track!

[myasonik]

jenkins test this

[kibanamachine]

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

[hetanthakkar]

@thompsongl can you please review

[thompsongl]

LGTM; Thanks for sticking with all the change requests!
Let's have @myasonik take another look before merging