Components: Fix no-node-access in Grid tests

[tyxla]

What?

With the recent work to improve the quality of tests, we fixed a bunch of ESLint rule violations. This PR fixes a few (10) no-node-access rule violations in the Grid component.

Why?

The end goal is to enable that ESLint rule once all violations have been fixed.

How?

We're adding a role to the test fixtures in order to be able to use screen.getByRole().

Testing Instructions

Verify all tests still pass.

[codesandbox]

  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

[tyxla]

I was contemplating whether it made sense to add the grid role by default to <Grid /> and then the gridcell role to each child view, but then I realized it probably won't always be the case that this makes sense. So it maybe makes more sense for the component consumers to add those roles as they see fit. Would love to hear some thoughts about this though!

[ciampo]

Reasoning makes sense to me — Grid is mostly about a visual grid layout, and it doesn't necessarily imply the semantics of role="grid".

For that reason, I'm not sure it makes much sense to add role="grid" to every test — on one hand, it allows us to use *ByRole queries, but on the other hand it feels a bit hacky.

I wonder if just passing a data-test-id to Grid is a better indication of our intensions in these tests?

[tyxla]

I never imagined I'd be reverting from using role to data-testid 😅 But yup, I agree that data-testid expresses our intent better. Updated in f47bd07

[ciampo]

I just think that role needs to be used with intention :) In this case, if we were using role as a "proxy" for the Grid component, that was mostly a hack

It's tricky to test pure layout components (like grid) semantically, which is why I used to mostly rely on snapshot testing in those scenarios (i.e. I mainly want to test that markup and styles are correct, no need to check for semantics).

[ciampo]

🚀