Table updated tests

[shaneeza]

✍️ Proposed changes

🎟 Jira ticket: Name of ticket

✅ Checklist

For bug fixes, new features & breaking changes

• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• I have run yarn changeset and documented my changes

For new components

• I have added my new package to the global tsconfig
• I have added my new package to the Table of Contents on the global README
• I have verified the Live Example looks as intended on the design website.

🧪 How to test changes

[tsck]

I wish there was a better way to handle these, but I can't come up with a straightforward way either =(

[tsck]

Nit: Wonder if it's necessary to check if this is in the document? It's not actually an implementation detail since we're rendering it directly in the test. Won't comment this on each one to bloat the review, but have the same thinking for each of these styled tests.

[shaneeza]

i think i can get rid of it since the expect below is also checking for it

[tsck]

As we discussed yesterday, I wanted this think about this some more but I think this is the correct call. We don't specify WithoutRT anywhere else, so I think this makes sense.

One question - it doesn't seem like all of these tests were moved over. Was that intentional?

[shaneeza]

Yes. I removed the tests for the zebra stripping because that seems more of a chromatic test. I thought we had a generated story to test zebra stripes, but it looks like we don't, so I'll update the generated row story to include it.

[tsck]

That makes sense to me!

[tsck]

Suggested change

	describe('packages/table/RowWithoutRT', () => {
	describe('packages/table/Row', () => {

[tsck]

Any reason for using fireEvent instead of userEvent here?

[shaneeza]

no, it was just a copy-paste; I'll update it.

[tsck]

If I'm correctly understanding why we're using 3 here, we could map this number more directly to the source here.

Suggested change

	const { result } = renderHook(() =>
	useLeafyGreenTable({
	data: getDefaultTestData({
	// eslint-disable-next-line react/display-name
	renderExpandedContent: (_: LeafyGreenTableRow<Person>) => {
	return <>Expandable content test</>;
	},
	}),
	columns: getDefaultTestColumns({}),
	}),
	);
	expect(result.current.getRowModel().rows.length).toEqual(3);
	const data = getDefaultTestData({
	// eslint-disable-next-line react/display-name
	renderExpandedContent: (_: LeafyGreenTableRow<Person>) => {
	return <>Expandable content test</>;
	},
	});
	const { result } = renderHook(() =>
	useLeafyGreenTable({
	data,
	columns: getDefaultTestColumns({}),
	}),
	);
	expect(result.current.getRowModel().rows.length).toEqual(data.length);

[tsck]

Ditto

Suggested change

	const { result } = renderHook(() =>
	useLeafyGreenTable({
	data: getDefaultTestData({
	subRows: getDefaultTestData({}),
	}),
	columns: getDefaultTestColumns({}),
	}),
	);
	expect(result.current.getRowModel().rows.length).toEqual(3);
	const data = getDefaultTestData({
	subRows: getDefaultTestData({}),
	});
	const { result } = renderHook(() =>
	useLeafyGreenTable({
	data,
	columns: getDefaultTestColumns({}),
	}),
	);
	expect(result.current.getRowModel().rows.length).toEqual(data.length);

[tsck]

Very nit =)

Suggested change

	// @ts-expect-error - requires columns, data, containerReff
	// @ts-expect-error - requires columns, data, containerRef