change default InMemoryTable sorting behavior

[lizozom]

Summary

This PR changes the default InMemoryTable sorting logic.
It now uses a three way sort on a field ASC -> DESC -> neutral

Blocker for EUIfication of visualizations \ dashboards screens on Kibana

Checklist

• This was checked in mobile
• This was checked in IE11
• This was checked in dark mode
• Any props added have proper autodocs
• Documentation examples were added
• A changelog entry exists and is marked appropriately
• This was checked for breaking changes and labeled appropriately
• Jest tests were updated or added to match the most common scenarios
• This was checked against keyboard-only and screenreader scenarios
• This required updates to Framer X components

[cchaos]

Congrats on your first EUI PR! 😆

As a quick etiquette rule for our PR's, the PR template that has the checklist needs to be kept in the PR summary. It helps remind of all the things that we need to add alongside what the PR is actually addressing like a changelog entry and adding/updating any tests. You can strike out any items that don't make sense though. But we do need keep the whole thing in the summary.

[thompsongl]

Thanks! Could you update the comment regarding initial sorting behavior and client sort logic? The statements are now misleading, given your changes.

eui/src/components/basic_table/in_memory_table.js

Lines 359 to 362 in 4e08b08

	// Data loaded from a server can have a default sort order which is meaningful to the
	// user, but can't be reproduced with client-side sort logic. So we allow the table to display
	// rows in the order in which they're initially loaded by providing an undefined sorting prop.
	// Once a user sorts a column, this will become a fully-defined sorting prop.

[lizozom]

@thompsongl done

[cchaos]

I'm not sure where the aria-labelling is hooked up, but it now needs to account for this new behavior, but only if that flag is used.

[thompsongl]

I'm not sure where the aria-labelling is hooked up, but it now needs to account for this new behavior, but only if that flag is used.

This is all done in table_header_cell.js. I actually think the aforementioned flag will be required so it can be passed down into the composition components.

[lizozom]

@cchaos I updated the aria labels and added a flag so that BasicTable could support both modes (with and without neutral sort).

I didn't expose the flag in the InMemoryTable api yet. Should I? And if so, what should be the default?
Since we want InMemoryTable to showcase our common behavior, shouldn't we simplify it by choosing one?

[cchaos]

Looks great! Thank you for fixing the screen reader stuff.

I just had some comments about making the prop on InMemoryTable overridable, and adding prop documentation.

You'll also want to be sure to rebase with master and create a changelog entry.

[lizozom]

@cchaos done review
Please check if I did the documentation right.

[lizozom]

@cchaos @thompsongl we're good to merge?

[cchaos]

Looks like a type in CL

[cchaos]

Docs look good, ty! I just saw a typo in the CL but other than that LGTM!

[thompsongl]

allowNeutralSort is still being passed through to the EuiBasicTable div element.
It's causing Warning: React does not recognize the 'allowNeutralSort' prop on a DOM element console warnings.

You'll need to remove it from the rest object before it gets spread onto the base div in basic_table.js

Note: You don't see this happen when allowNeutralSort uses the default value. It occurs when explicitly setting the prop on EuiInMemoryTable

[lizozom]

Once it's in master, how can I start using it in Kibana?

[chandlerprall]

@lizozom as soon as #1619 is merged we'll make an EUI release which will include these table changes. We'll create a PR in kibana to bring the latest EUI into master, and backport that into the 7.x branch.