[DataGrid] Enhance comparator signature to support advanced use cases

[dej611]

Summary

As discussed in #6867 (comment) added a new argument with row idex information for accessing raw values to compare.

[cee-chen]

👏 Huzzah, looks awesome to me! Love the added arg being an object for future flexibility/extension if needed, and I really like that we opted to pass .index instead of .values (initially that what was I was looking at, but I feel like .index is more lightweight).

Add a unit test and a changelog and I think this is good to go! Let us know if you'd like any help with that, I'm happy to push up to this branch.

[kibanamachine]

Preview staging links for this PR:

• Docs site: https://eui.elastic.co/pr_7406/
• Storybook: https://eui.elastic.co/pr_7406/storybook

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 08284c5

History

• 💚 Build #1161 succeeded f420243
• 💔 Build #1154 failed 3ae64e8
• 💔 Build #1151 failed 861fde2

[JasonStoltz]

I'm good with these changes 👍

[JasonStoltz]

@cee-chen If you have any feedback regarding the tests, we can do a follow-up PR, but they looked good to me. I'll merge this in.

[cee-chen]

Thanks for creating this test file! ❤️ You rock Marco, the test looks really great!

[cee-chen]

I know the PR has already merged, but just curious - from a developer experience perspective, do you think the type/prop docs are sufficient, or should we add/update an existing EuiDataGrid documentation example as well?

[cee-chen]

Ah I might actually open a new PR for this. I think we should have typed this as

Suggested change

	indexes: { aIndex: number; bIndex: number }
	optional?: { aIndex: number; bIndex: number }

to make it clearer it's extra information that's optional for developer usage

[JasonStoltz]

🤦 Jumped the gun on the merge, my bad!

[dej611]

As a developer I read that as optional can either be passed or not, please check.
I think it make sense the renaming (indexes => optional) but the optional type (optional?: )is actually misleading there. It makes more sense to keep it without the optional to communicate to the developer that he might use it or not, but it will always be passed.

A similar approach is used also for all JS Array methods, i.e. Array.prototype.filter:

filter(predicate: (value: T, index: number, array: T[]) => unknown, thisArg?: any): T[];

The array value there is not often used, but it will be always passed no matter what predicate function is defined. The optional type is applied only to the thisArg, as it can be available depending on the actual usage.

[cee-chen]

Ahh yeah that's a super good point about the ts ?, thanks for catching that! Just renaming the arg to optional for documentation & future flexibility should be fine in that case