[kbn-grid-layout] Add ability to create, edit, and delete rows

[Heenawter]

Closes #204849

Summary

This PR adds the ability to create, edit, and delete sections / rows to kbn-grid-layout:

Screen.Recording.2025-02-12.at.12.38.59.PM.mov

Note that sections are still statically placed - dragging rows around will be added in a follow-up PR, because it's a larger undertaking. Since this feature is not available to users yet, it is okay to implement this in stages like this.

Checklist

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Unit or functional tests were updated or added to match the most common scenarios
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Collapsible sections are not available on Dashboard yet and so there is no user-facing risk to this PR.

[Heenawter]

@ThomThomson

When I delete a section, and select "delete section only" it scrolls me to the bottom of the Dashboard. Is this intentional? Or a consequence of the way the scroll when a section is added logic works?

I'm not seeing this 🤔 Can you explain how to repro this?

Screen.Recording.2025-02-21.at.3.31.48.PM.mov

The section state is not backed up into session storage in the example like the state of the panels. When I create a new section and put panels into it then reload the example, that section is completely gone. Is this something you intend to add in a followup?

State is only backed up when you save the example - not on reload (this applies to panels too). Maybe I'm missing something, but I am able to save sections and they are restored as expected?

Screen.Recording.2025-02-21.at.3.35.21.PM.mov

[ryankeairns]

@Heenawter a) this is awesome and b) I have suggestions for that modal having just drafted some button guidelines.

If possible, can we change the paragraph text and buttons to this design:

This order, text, and combination of styles better clarifies the actions of each button and increases the prominence of the most destructive action.

[Heenawter]

@ryankeairns Love it! Done in f5c90e6

[mbondyra]

Left a few nits, but nothing blocking. Great job! 👏🏼

[mbondyra]

Also nit and something we can improve later on: I think that when creating wrapping functions on top of API that is well known and widely used, it's good not to change this api, but only add new methods/properties specific to what we need for the specific usage.

Suggested change

	return {
	component: render(
	<GridLayoutContext.Provider
	value={
	{
	renderPanelContents: mockRenderPanelContents,
	gridLayoutStateManager: stateManagerMock,
	...contextOverrides,
	} as GridLayoutContextType
	}
	>
	<GridRowHeader
	rowIndex={0}
	toggleIsCollapsed={() => toggleIsCollapsed(0, stateManagerMock)}
	collapseButtonRef={React.createRef()}
	{...propsOverrides}
	/>
	</GridLayoutContext.Provider>,
	{ wrapper: EuiThemeProvider }
	),
	gridLayoutStateManager: stateManagerMock,
	};
	const rtlRender = render(
	<GridLayoutContext.Provider
	value={
	{
	renderPanelContents: mockRenderPanelContents,
	gridLayoutStateManager: stateManagerMock,
	...contextOverrides,
	} as GridLayoutContextType
	}
	>
	<GridRowHeader
	rowIndex={0}
	toggleIsCollapsed={() => toggleIsCollapsed(0, stateManagerMock)}
	collapseButtonRef={React.createRef()}
	{...propsOverrides}
	/>
	</GridLayoutContext.Provider>,
	{ wrapper: EuiThemeProvider }
	)
	return {
	...rtlRender,
	gridLayoutStateManager: stateManagerMock,
	};

This way, we can access the original RTL render methods in the same way as always while also accessing gridLayoutStateManager directly. No need to mentally map transformed methods—everything stays familiar.

So we can still do:

const { getByTestId, gridLayoutStateManager } = renderGridRowHeader();
    const initialCount = getByTestId('kbnGridRowHeader-0--panelCount');

But actually a better practice is to use screen exported directly from rtl package: https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#not-using-screen since we don't have to keep up with destructuring.

    const { gridLayoutStateManager } = renderGridRowHeader();
    const initialCount = screen.getByTestId('kbnGridRowHeader-0--panelCount');

[Heenawter]

I personally like keeping the Kibana-specific stuff (gridLayoutStateManager in this case) separate from the RTL stuff, but maybe that's just me 🙈 Agreed that we could clean this up by using screen - it's an old habit of mine to use the returned component, oops. We can clean this up in a follow up.

[mbondyra]

Hmmm, Maybe I am missing something, but I feel quite strongly about this approach. Keeping the original API while adding necessary extensions is the recommended pattern for RTL decorators (I took the testing course from the library creator 😁) . Beyond the benefit of using render in a familiar way, there are more advantages:

1. Composability – If another helper like renderWithProviders is introduced and we want to use one function on top of another, its return shape remains consistent, avoiding unnecessary checks for the proper shape. So we don't have to check in the more nested function if what we process is an object with {component: rtlReturn} or just rtlReturn (and so on with more nestings).
2. Maintainability – If we later remove a wrapper, we can revert to original render method without modifying tests.
3. I don't see any advantage to keeping the usage specific stuff separate from rtl stuff, unless you fear you can accidentally override it. The thing is, if you do, it's probably intentional and makes things even more elegant (like overriding the rerender function). Plus it's not really component render function returns, so choosing a name here is also tricky.

Maybe there's something I don't see about your preference. I am happy to discuss it within the team if we want to make a 'goto' decision. This is the pattern I introduced in Lens codebase and compared to what we had before (=not caring about keeping the original API) it made our life so much simpler that I am very enthusiastic about it.

[Heenawter]

I don't have a super strong preference so I'm good either way, since it's primarily a style choice! I was just following the standard we had in other Presentation tests, but your reasoning (especially point 2) sounds good enough to me to make the change in a follow up :)

[kibanamachine]

Starting backport for target branches: 9.0

https://github.com/elastic/kibana/actions/runs/13505149060

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 18f090d

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
dashboard	740	743	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
dashboard	535.3KB	541.8KB	+6.5KB

History

• 💚 Build #278830 succeeded b94d5a0
• 💚 Build #278625 succeeded f5c90e6
• 💚 Build #277885 succeeded a90288a
• 💛 Build #277824 was flaky 253c9f4
• 💚 Build #277464 succeeded 8150dd6
• 💔 Build #277385 failed dc2c369

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	9.0	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 209193

Questions ?

Please refer to the Backport tool documentation

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/13507039876

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation