Fix regression on keyboard navigation

[oandregal]

What?

This PR fixes a regression on keyboard navigation for the list layout (dataviews) by which the list items weren't reachable via TAB.

Gravacao.do.ecra.2024-05-08.as.10.56.29.mov

How?

The result of getItemDomId needs to be undefined if there's no item.

Testing Instructions

• Visit Site Editor > Pages.
• Try to reach the list items via keyboard navigation (TAB).

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: oandregal <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: gziolo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[oandregal]

What's the proper type here?

[youknowriad]

By reading the code it seems to be "string" but optional, so something like

Suggested change

	id: any;
	id?: string;

[gziolo]

string | undefined?

[sirreal]

I'll add a bit of information, there's a subtle difference in what these two communicate:

// The property may not be present. If it is present it's a string
var x: { id?: string } = {};
var x2: { id?: string } = { id: undefined }; // TYPE ERROR!!
// The property _is_ present, `undefined` is an acceptable value. {} is not allowed here.
var y: { id: string | undefined } = { id: undefined };
var y2: { id: string | undefined } = {}; // TYPE ERROR!!

(Note that the x2 type error depends on exactOptionalPropertyTypes which I don't think we have enabled in Gutenberg right now. It's relatively new but likely a good thing to enable.)

At runtime we often treat these as the same, but they're also subtly different depending on the type of check we're doing:

const x = {}
x.y; // undefined
Object.hasOwn(x, 'y'); // false
'y' in x; // false

x.y = undefined;
// The value is the same but the property is there:
x.y; // undefined
Object.hasOwn(x, 'y'); // true
'y' in x; // true

[youknowriad]

We should think about adding some more e2e tests for data views (keyboard navigation being one of them)