[DataGrid] Expandable rows

[ryankeairns]

On occasion, teams have expressed a need for expandable rows in the data grid. While not depicted in this exact PR, the future desire here is to expose additional metadata in a row while taking advantage of the other pros of using data grid (i.e. scalability, column selection, advanced sorting, etc.). Also in the area of alerting, the Security team has expressed similar desires for an expandable row.

As the topic has come up more than once, we are curious to get the EUI team's take on possibility of adding this feature to the data grid.

• If others have designs to add, please do so!
• If non-EUI engineers are willing to offer assistance in this effort, please speak up!

cc:/ @gmmorris @mdefazio

[chandlerprall]

🚀 We are currently putting together a spec for how expandable rows could/should work, and expect to have that ready for review some time next week. Once everyone agrees on the spec we can figure out how to split up implementation efforts.

[ryankeairns]

Any thoughts on what we do next here?

My personal thoughts are to capture a clear set of requirements from the teams involved. DeFazio and I are actively working with ResponseOps and can start putting something together with their guidance (doc or description here). It looks like the Security team has already provided a fair bit of feedback on this this discussion, but somebody from that team could clarify their reqs, as needed.

[snide]

This needs a requirements spec from the downstream teams. I'd asked @adamwdraper and @dimadavid to solidify this from the Security mindset. @ryankeairns if you can get them engaged and start a real document to give to the EUI team that would be helpful. As far as I know this hasn't been picked up and while we're aware of the needs of the current Security app, we're still a bit in the dark for what they'd need go forward (which is based on the assumption that both the timeline and alert views have changes coming).

[cee-chen]

UX questions

1. If a data grid has expandable rows, can we assume that every row in that grid will be expandable? Or do we want to support only specific rows to be expandable (e.g. passing a config map of row indices to isExpandable booleans?)

2. Do we need to support expanding multiple rows at a time? Or will this behave like an accordion, where expanding one row closes all others? (Assuming multiple rows, as this is how EuiBasicTable currently works)

3. Do we need the ability for consuming applications to control the expanded state of row's outside of the user toggles/internal state? For example, do we need the ability for a data grid to load in with a specific row already expanded? Or to for an application to control hiding a row (e.g.) when a modal opens?

4. Does the toggle icon/location need to be customizable? Or similar to the current EuiBasicTable implementation, can we say that the toggle button will always be an up/down arrow, and the last column of every row (behind all trailingControlColumns)?

   • Or, should we allow consumers to customize the rendering and of the toggle button by passing it in themselves to either a leadingControlColumn or trailingControlColumn? (adds significant complexity)

[cee-chen]

That's super interesting, thanks Andrew. So to be clear, you're saying this expanded row behavior isn't really similar at all to the accordion-like behavior of https://elastic.github.io/eui/#/tabular-content/tables#expanding-rows - the expanded content is always present and cannot be collapsed? Is that correct?

@cchaos mind if I loop you in on this since you just spoke yesterday about keeping EuiBasicTable and EuiDataGrid more 1:1? Is this a difference in UX between the 2 components we're comfortable implementing?

[andrew-goldstein]

So to be clear, you're saying this expanded row behavior isn't really similar at all to the accordion-like behavior of https://elastic.github.io/eui/#/tabular-content/tables#expanding-rows - the expanded content is always present and cannot be collapsed? Is that correct?

Correct, the expanded content is always present, and cannot be collapsed. Some content, specifically notes, can be added dynamically, like adding feedback to code in a Github PR.

[cee-chen]

Gotcha!! That's totally my bad for assuming (I heard "expandable" and assumed an accordion, but this is really more like "extra associated row data" or "always expanded"). That does simplify implementation thankfully, rather than the opposite. I can revise my technical approach post below once we get sign-off from the designers that this deliberate UX differentiation in the data grid is OK.

[cchaos]

Hey @andrew-goldstein thanks so much for your details and feedback on your use-case. Knowing the technical challenges of what you're asking for and the implications on the usability of the EuiDataGrid, I'd like to put a non-collapsing details row on the back burner until we have a better idea of the future of the timeline grid. I know that there are some overall UX improvements that will be looked into in the near future by the security designers, which would then feel like a better time to approach technical needs.

[ryankeairns]

As this topic continues to come up in our interactions with the ResponseOps team, and we learn more about their thoughts and use cases, I would answer "yes" to all four of the questions in atop this thread.

The use case of presenting errors in the rule activity log is the most recent example. The log (presented in the data grid) has one row for every run; each run can have multiple errors; keeping that association is desired; external toggles and linking into this table (i.e. starting with rows expanded) are needs that have been discussed.

The requirements are, in my opinion, still a bit loose. This may be more of an issue of them still having open positions for PMs rather than uncertainty of needs. cc:/ @XavierM

[cee-chen]

API questions/proposals

What should the DataGrid's API for this look like? A couple examples/proposals off the top of my head:

1. Object map

<EuiDataGrid
  expandedRowMap={{
    0: <SomeExpandedContent />,
    2: <OtherExpandedContent />,
  }}
/>
// This feels similar to EuiBasicTable's `itemIdToExpandedRowMap`
// Is this too simplistic? No way of passing props to the expanded rows. Or maybe that isn't necessary?

2. Render fn/component

<EuiDataGrid
  renderExpandedRow={({ rowIndex, isExpanded }) => {
    if (rowIndex === 3) return <SomeExpandedContent />;
    return <OtherExpandedContent />;
  }}
/>
// Similar approach to `renderCellValue` and `renderCellPopover` fns, allows us to pass props/info a component passed by the consumer

3. Something else I've overlooked? Leave a comment!

[andrew-goldstein]

At a glance, Option 2, the render function, feels like a better fit for the use cases we have in mind

[andrew-goldstein]

The current row's data should also be available as a prop in option 2

[cee-chen]

The data grid has no concept of data and cannot (easily) return that to you. All we're doing is rendering the fn/component the consuming application has passed via renderCellValue. If we go with renderExpandedRow, you should however have the same access to your own data (given that we send back the rowIndex) and be able to access it the same way you do in renderCellValue.

CC @chandlerprall on this one, definitely feel free to correct me if I'm wrong. It's possible this might be doable with an inMemory level set, but then we'd need to require that prop while using renderExpandedRow which I feel a bit medium about.

[chandlerprall]

The data grid has no concept of data and cannot (easily) return that to you.

++

Even if we went the inMemory route, the data the grid is privy to isn't the underlying data, and the app would need to somehow parse the rendered content back into whatever they need to use. Definitely agree that the expanded row renderer should re-connect the any data store for its content.

[andrew-goldstein]

The data grid has no concept of data and cannot (easily) return that to you.

++

Even if we went the inMemory route, the data the grid is privy to isn't the underlying data, and the app would need to somehow parse the rendered content back into whatever they need to use. Definitely agree that the expanded row renderer should re-connect the any data store for its content.

At a glance, I thought data was missing as a prop to render because I was thinking about the EuiBasicTable's render callback in EuiTableFieldDataColumnType, where render includes the data (per the docs):

(value: any, record: T) => ReactNode

but as noted above data grid isn't privy to the data.

The rowIndex prop in option 2 is sufficient to index into the underlying data (which is not managed by Data Grid).

[cee-chen]

Accessibility questions

CCing @1Copenut on this for any expertise he might have on tables/grids with expanded rows and if W3 has a spec for it & how they should behave for screen readers.

Expanded row behavior is somewhat broken in EuiBasicTable - once a row is expanded, its original data cells can no longer be reached by SR arrow navigation. I strongly feel that if we implement expanded rows in EuiDataGrid, they should be fully keyboard and screen reader accessible; we have worked hard as a team to continuously improve data grid's accessibility and I'd really like to keep it that way.

1. What role does this expanded content have in relation to the grid? Do we give it a role="gridcell" and treat it like an extra cell that belongs it to the row that happens to have extra information? Or do we give it a role="row" and treat it like its own completely separate row with only 1 cell?

2. Can we agree that when expanded, both the original row that toggled the expansion and the expansion content itself should remain fully navigable by both keyboard and screen reader users?

   • I'm looking at this broken behavior in https://elastic.github.io/eui/#/tabular-content/tables#expanding-rows and it looks like the culprit there may be aria-owns on the first <tr> - any thoughts on whether we need this attribute @1Copenut or can we get away with pretending the expansion row is an extra gridcell instead?

3. Any other accessibility concerns that I might be missing?

[andrew-goldstein]

2. Can we agree that when expanded, both the original row that toggled the expansion and the expansion content itself should remain fully navigable by both keyboard and screen reader users?

Timeline's screen reader support includes the same content as Data Grid, with additional announcements + keyboard navigation guidance when a row render or notes are present. We could consider a similar approach in Data Grid:

When rows do NOT have a renderer, Timeline announces the following via voiceover, which is similar to Data Grid:

• "You are in a table cell"
• The cell row and column number
• The contents of the cell, in context of the column (e.g. event.category -> network)
• Additional drag and drop instructions

as shown in the screenshot below:

Above: voiceover support for a row with NO renderers in Timeline is similar to Data Grid

When a row has additional content, the announcement includes additional content:

• It announces a row renderer and / or notes exist
• It tells the user how to access the renderer or notes via keyboard

as shown in the following screenshot:

Above: Via the screen reader, Timeline announces additional row content, and tells the user how to access it via keyboard

Un-mute the video below hear what this sounds like:

SOUND_ON-row-renderer-announcement.mov

Above: SOUND ON: Un-mute the video above to hear voiceover announce an event renderer

Focus trapping renderers via EuiFocusTrap for Tab navigation

When a renderer is focused:

• The screen reader announces "You are in an event renderer for row n"
• The screen reader announces how to escape the renderer via the keyboard
• EuiFocusTrap traps keyboard focus within the renderer for Tab navigation

Here's a sample announcement:

Above: Timeline announces when a renderer has trapped focus, and provides instructions on how to escape the renderer

Un-mute the video below hear what this sounds like:

SOUND_ON_entering_a_row_renderer.mov

Above: SOUND ON: Un-mute the video above to hear voiceover announcements when an event renderer owns keyboard focus

[cee-chen]

Fantastic! Thanks for the incredible detail and aria spec links, Andrew!

One quick thing worth noting, WAI-ARIA spec is now on 1.2 and we just spent some time updating our comboboxes from 1.1 to 1.2 spec. It may be worth updating our data grid references to 1.2 as well: https://www.w3.org/TR/wai-aria-practices-1.2/examples/grid/dataGrids.html (although it also looks like there isn't an official spec for expanded rows, so may not be super applicable to this convo)

Regarding shift arrows+focus trap+tab in the expanded row content - I'm not opposed to it, but I also wonder if it wouldn't be easier to simply treat the row as a "normal" cell and let keyboard/screen reader users use the standard up/down arrow keys to navigate to it (and then pressing the enter key to enter the cell's focus trap). After all, expanded rows are very readily available to sighted users - why make it slightly more difficult than normal to get to for non sighted/mouse users?

For an example of how I would envision the focus behavior for keyboard users, see the column with the pink "2 interactive" header in https://elastic.github.io/eui/#/tabular-content/data-grid-focus.

[andrew-goldstein]

Thanks for your feedback @constancecchen!

Regarding shift arrows+focus trap+tab in the expanded row content - I'm not opposed to it, but I also wonder if it wouldn't be easier to simply treat the row as a "normal" cell and let keyboard/screen reader users use the standard up/down arrow keys to navigate to it (and then pressing the enter key to enter the cell's focus trap). After all, expanded rows are very readily available to sighted users - why make it slightly more difficult than normal to get to for non sighted/mouse users?

Likewise, I'm not opposed to representing additional content as "just another row", with a single column that spans the width of the table.

The main drivers of the current approach were to:

• Semantically preserve a 1:1 relationship between rows of (source) data and table rows with Aria role="row"
• Have an explicit boundary where focus is trapped, such that continuously pressing tab eventually cycles back to the first item in the role="dialog" content

I think the focus-trapping concerns in the 2nd bullet point above are satisfied by the behavior in the example you provided:

For an example of how I would envision the focus behavior for keyboard users, see the column with the pink "2 interactive" header in https://elastic.github.io/eui/#/tabular-content/data-grid-focus.

[1Copenut]

I've been working on this for a bit, both testing the existing tables with expandable rows, and coding my own mockup to test. I'll answer @constancecchen questions inline.

1. What role does this expanded content have in relation to the grid?
   I think we should keep the expanded content as a more generic "group" or possibly a "region". Region feels too formal for what the data is representing. It generally wouldn't stand on its own outside of the table/grid row. I don't think we're going to be able to use a role="dialog" and meet the stated goal of having multiple rows expanded. Dialogs assume other content is inert and could create a confusing experience for assistive technology users.
2. Yes, the entire row of cells, and the expanded content should be fully navigable by keyboard and by extension, screen readers. I quick tested the existing EUI tables with expandable rows in Safari + VO. It does appear there's a bug in the navigation when rows are expanded, so I'll file a separate backlog item for that.
3. The only other item I know we'll need to discuss will be the keyboard navigation paradigm for data grids. Users can traverse in all directions using arrow keys, but how do we reconcile that pattern with an accordion-like expanded content block? This is an open question more than a definitive solution.

[cee-chen]

Adding some extra accessibility notes from my POC spike:

role=dialog is definitely incorrect from my research of aria roles. role=note or region might be more applicable, but to be honest, I really think the 'expanded row details' should just be considered a role=gridcell within the existing table row that just happens to be positioned/sized differently, i.e., the markup of https://fuschia-stretch.glitch.me

[cee-chen]

Technical approach & suggestions

Virtualization

To fully understand the technical challenge of expandable rows, you must first understand how Data Grid's virtualization library (react-window) works. I recommend taking a look at their VariableSizeGrid example, scrolling around, and inspecting the DOM of each cell. Once you do, several things become apparent:

1. Each cell is absolute positioned and has its position, height, and width set with inline styles that come in from react-window.

2. What this means is we cannot simply pass in a height override to a row and call it a day.

   • If we do so without interacting properly with react-window's API, what will happen is the rows below the expanded row will not reposition themselves accordingly, and we'll get some visually buggy overlapping row jank.

3. So to do this correctly, we'll need to interact with react-window's rowHeight prop and get react-window to account for the expanded row height in order for all other rows to shift/adjust accordingly. Here's EUI's current logic/function that we pass into react-window's rowHeight prop:
   

   eui/src/components/datagrid/utils/row_heights.ts

   Lines 306 to 336 in dff7a77

   	// Used by react-window's Grid component to determine actual row heights
   	const getRowHeight = useCallback(
   	(rowIndex: number) => {
   	const correctRowIndex = getCorrectRowIndex(rowIndex);
   	let rowHeight;
   	// Account for row-specific height overrides
   	const rowHeightOption = rowHeightUtils.getRowHeightOption(
   	correctRowIndex,
   	rowHeightsOptions
   	);
   	if (rowHeightOption) {
   	rowHeight = rowHeightUtils.getCalculatedHeight(
   	rowHeightOption,
   	minRowHeight,
   	correctRowIndex,
   	rowHeightUtils.isRowHeightOverride(correctRowIndex, rowHeightsOptions)
   	);
   	}
   	// Use the row-specific height if it exists, if not, fall back to the default
   	return rowHeight || defaultRowHeight;
   	},
   	[
   	minRowHeight,
   	rowHeightsOptions,
   	getCorrectRowIndex,
   	rowHeightUtils,
   	defaultRowHeight,
   	]
   	);

4. The above code should already account for several different row height options (single rows, line count row heights, and auto row heights). What we'll need to do is to find the height of the expanded content and add that to the existing row's height.

   • My initial instinct would be suggest measuring each row's expanded content just-in-time (i.e. when the user clicks the "expand row" toggle button) rather than attempting to measure all hidden expandable content on data grid load and dealing with a store/cache.
   • This does depends on a few things, however - do we plan on having images that will load in slowly and throw off/change the height of the expanded row content? If so, that will increase the complexity of this a good amount.

5. However, updating rowHeight will change the style.height property coming from react-window to each cell in that row:
   

   eui/src/components/datagrid/body/data_grid_body.tsx

   Lines 113 to 116 in 93b70f9

   	style: {
   	...style,
   	top: `${parseFloat(style.top as string) + headerRowHeight}px`,
   	},

   
   And visually, we don't want every single cell in the row to match the height of the expanded row. We want them to retain their old pre-expanded heights, and only the expand control column and the row element should expand their heights:
   

   eui/src/components/datagrid/body/data_grid_cell.tsx

   Lines 700 to 705 in 93b70f9

   	rowManager.getRow({
   	rowIndex,
   	visibleRowIndex,
   	top: style!.top as string, // comes in as a `{float}px` string from react-window
   	height: style!.height as number, // comes in as an integer from react-window
   	})

6. To work around this, we'll need to adjust the style.height property going to each cell to account for expanded vs. non-expanded state, and pass them their correct non-expanded heights so they retain their current/previous heights. This could look something like this, on data_grid_body.tsx#L115:

   style: { 
     ...style, 
     top: `${parseFloat(style.top as string) + headerRowHeight}px`, 
     height: isRowExpanded ?
       ? style.height - rowExpansionContentHeight,
       : style.height,
   }, 

   • Minor caveat: since the row manager inherits height data from its cells, we either need to re-offset the row height passed on data_grid_cell.tsx#L704, or we need at least 1 cell (likely the expansion control cell) to retain the full expanded row height.

Rendering

OK, now that our row & cell heights and positions are in the right ballpark of where we want them, we now need to render the row expansion content itself.

My first instinctual approach would be to bundle the row expansion toggle and the row expansion content together within one "cell". The alternative would be to attempt to render the expansion toggle & content as 2 separate cells, which has potentially gnarly side effects on the underlying grid (column counts are now "slightly" off by 1 invisible 'column', which can then affect keyboard navigation, etc) so if possible I'd rather try to keep the toggle (visible cell) and the content (conditionally hidden) conceptually together.

Here's a rough visual mock of what I'm thinking, based on EuiBasicTable:

Moving ahead with the bundled approach: if we treat the row expansion toggle as a special trailingControlColumn (depending on UX question 4), the logic where we handle rendering for this will likely be in data_grid_body.tsx's Cell function component (after the else if (isTrailingControlColumn) block). We will likely need to put a special className on that cell (to override some default CSS positioning). The necessary DOM and CSS I'm imagining is something like this:

Let me know if that doesn't make sense. Again, I think this is the simplest way to handle rendering the expanded content, but I could be off the mark here for a few reasons (DOM complications, accessibility considerations, etc.), so this may be the area we most need to revisit or troubleshoot.

State

OK, so now we have virtualization positioning and rendering (sort of) out of the way - how do we know which row(s) should be open and which should be closed? Assuming multiple rows can be expanded (see UX question 2), I would suggest an expandedRowMap state that looks something like this: { 0: true, 1: false, 2: true } (leaving indices as undefined, i.e. initially closed, and only setting to false after close should be fine), e.g.

const [expandedRowMap, setExpandedRowMap] = useState({ 0: true }); // Maybe allow passing an initialExpandedRowMap?
const expandRow = useCallback((rowIndex: number) => {
  setExpandedRowMap((expandedRowMap) => { ...expandedRowMap, [rowIndex]: true });
}, []);
const collapseRow = useCallback((rowIndex: number) => {
  setExpandedRowMap((expandedRowMap) => { ...expandedRowMap, [rowIndex]: false });
}, []);

This state must exist at the EuiDataGridBody level or higher and not at the individual cell level (in order for every cell to know if its parent row is expanded or not).

If we need to expose some sort of expandRow(rowIndex) or collapseRow(rowIndex) API as a ref method (see UX question 3), then we should set this up at the topmost EuiDataGrid level instead, likely using a separate util file/hook that passes the necessary state/setters down to useImperativeGridRef() and to EuiDataGridBody. (useCellPopover is an example of a separate hook util that handles its own internal state and waterfalls the results back down to individual cells).

Also worth noting and QAing for - we should ensure we are using the rowIndex (index from user data) at all times and not visibleRowIndex (which is affected by sorting and pagination). To this end, we should ensure we QA on sorted & paginated data grids and ensure everything still works even when a row is moved via sorting.

Questions?

Have I missed a consideration (very likely)? Leave a comment if so!

[1Copenut]

Opened #5673 to track refactoring effort for EuiTable expanding rows.

[chandlerprall]

We've since done a lot of work around dynamic row heights which could help provide an implementation path. Constance took some time to explore this again (branch) and found these concerns:

• height detection is occasionally problematic ([Security Solution] Extra space is displaying under alerts table when alerts are less than the number to which pagination count is set kibana#129057 and others), this should be resolved before we do any other height related changes
  • the author of react-window has an autosizer that may help resolve
  • react-window might be too limited to allow this functionality in a proper form
• additional code complexity & interaction with other height logic and virtualization work arounds demands attention and solid testing, test cases
• this proof-of-concept iteration has some lingering styling issues that were not investigated

With resource limitations, we have to shelve any further exploration. If anyone would like to pick this up let us know!

[1Copenut]

I'm working on a few things today so I can really focus on this discussion and offer useful a11y guidance. Hope to have that out before end of week.

[1Copenut]

@constancecchen

My first instinctual approach would be to bundle the row expansion toggle and the row expansion content together within one "cell".

I feel this is the right approach too. Semantically I don't think we're going to be successful conveying the idea the expanded content is related to the row vs. "part of" the row's information. Thinking out loud for a second, if a row in the data grid represents one log entry for instance, the expanded content could be more about the endpoint that spawned the log entry. It's more data about this one log instance, but doesn't tell us much about a log entry as a singular element.

I like how you called out the parent container, the expand/collapse trigger, and the content itself. This was helpful to me to form this ^^ idea about semantic relationships.

[ryankeairns]

@gmmorris I would be curious to hear your thoughts from the ResponsOps side regarding:

1. which data grid features are of particular interest to your area (e.g. adding columns, advanced sorting, cell actions, etc.) and
2. how you envision using the expandable rows

Perhaps there is an alternative approach here where the features you seek from data grid could be supported by existing table components which already support expandable rows... I'm envisioning the bulk edit bar enhancements. It seems we're all trying to find the line of what makes a data grid a data grid and where we may just be blending things to the point we lose their specialties.

[gmmorris]

which data grid features are of particular interest to your area (e.g. adding columns, advanced sorting, cell actions, etc.)

All of the above ;)
Basically, think of it this way:
We want a UX that will allow an administrator to look at a list of 100s of rules, filter them down to a subset that requires their attention and perform a bunch of operations on them.

For example, imagine the following scenario:

A user needs to temporarily mute all the rules that are monitoring a specific subset of systems because they're about to go into maintenance and the alerts could cause a whole range of problems (such as waking up on-call folks).
So they go to this list of 500 rules, type a filter in the KQL bar and realise that not all the rules relating to the system actually need to be disabled, only some, but they're not sure which. Luckily they know that it's only the high severity rules that are going to wake on-call folks up, so they sort by severity to identify the top ones, but some of the high severity ones might still need to remain enabled, so they want to look at the top rules to decide which.
This is where row expansion comes in - if they have to click into the rule (which navigates to a new page) to see the details then they lose their filtering and sorting, and there are dozens of rules, so this won't work. If the the table has column selection then maybe they can display the column with the differentiating data, but even then, some data might not be easy to display in a column. If they have row expansion they can expand the rule, evaluate, and if it's relevant - they can hit the check box to select it, close the expansion, and move on the to the next one.
Once all relevant rules were checked, they choose a bulk action to schedule a mute period, save the muting, and reset the filters.
They now sort by mute status and see that all the rule they were worried about are muted.

Once the work is done they come back to the table, sort by mute status, check and unmute these rules, and go to sleep.

This sort of workflow is super hard (and non performant) with in-memory tables, and would require us to develop a bunch of featured that are available in the data-grid but not the table.

Have I missed anything here @XavierM ? :)

[ryankeairns]

Thanks for writing up this use case, it is very helpful in understanding needs.

[cee-chen]

@andrew-goldstein @michaelolo24 - I know it's been... almost literally a year since we've talked about this (based on the GH discussion timestamps - although I know we've chatted about this more recently than that) - but I wanted to quickly highlight an open PR that will enable your team to layout your own custom expandable rows: #6624

Staging example: https://eui.elastic.co/pr_6624/#/tabular-content/data-grid-advanced#custom-body-renderer

Check the "toggle row details" switch:

The major tradeoff to this API is that you lose built-in virtualization (so may not be performant depending on the # of rows being displayed per page), and you are completely in charge of laying out cells. I've built the staging example to be as close to the usage that (I think) y'all wanted as possible, so that part hopefully shouldn't be a major issue, but I'd love to get feedback from your team and whether it will either meet or your needs or need to be tweaked before merging this in.

[andrew-goldstein]

Thank you @cee-chen, this is awesome! 🙌

🎉 The staging example feels identical to our use case!

📐 At a glance, it looks like having a variable table height and a variable detail row height are both supported by the new API

Q: Are there any issues if only some of the rows have a detail cell (as determined by the contents of the row data)?

⌨️ Keyboard navigation works as-expected, and enables the detail row to be focused

Q: Are there any issues if the contents of a details cell were wrapped in an EuiFocusTrap, to allow keyboard navigation between the focusable-content in the details cell?

The major tradeoff to this API is that you lose built-in virtualization (so may not be performant depending on the # of rows being displayed per page)

The virtualization provided by React Window doesn't easily integrate with RBD&D, so loosing built-in virtualization isn't a problem at all for our use case 😄

In summary, the new API appears to have everything we need to render Timeline content, including row renderers and markdown comments, in Data Grid!

[cee-chen]

Q: Are there any issues if only some of the rows have a detail cell (as determined by the contents of the row data)?

I think this would be an issue, primarily one of accessibility and keyboard focus/navigation. Right now this extra 'detail row' is actually a cell and thus still responds to the same arrow key navigation that a cell would. Suddenly losing a cell would break arrow key navigation and additionally be unexpected for non-sighted users who would expect a grid to stay relatively consistent.

If you absolutely must have conditional row details, my suggestion would be to conditionally add an .euiScreenReaderOnly CSS class to visually hide the cell but make its content SR text along the lines of, "This row has no additional details. Press the up or down arrow keys to see the previous or next row's details."

Q: Are there any issues if the contents of a details cell were wrapped in an EuiFocusTrap, to allow keyboard navigation between the focusable-content in the details cell?

If you are still using the underlying EuiDataGridCell component for the row details "cell", that focus functionality should come for free if 1. you have more than 1 interactive/tabbable child within the cell, and 2. your cell does not have an expansion popover. You can see this behavior in action in our prod docs:

• https://elastic.github.io/eui/#/tabular-content/data-grid-cells-popovers#focus
• Click the leftmost cell with two "Yes | No" buttons
• Press the Enter key to enter the cell focus trap
• Note that the user is now focus trapped & can tab/navigate between items within the cell

[andrew-goldstein]

Q: Are there any issues if only some of the rows have a detail cell (as determined by the contents of the row data)?

I think this would be an issue, primarily one of accessibility and keyboard focus/navigation. Right now this extra 'detail row' is actually a cell and thus still responds to the same arrow key navigation that a cell would. Suddenly losing a cell would break arrow key navigation and additionally be unexpected for non-sighted users who would expect a grid to stay relatively consistent.

If you absolutely must have conditional row details, my suggestion would be to conditionally add an .euiScreenReaderOnly CSS class to visually hide the cell but make its content SR text along the lines of, "This row has no additional details. Press the up or down arrow keys to see the previous or next row's details."

In our use case, only some rows will have content that can be visualized with a renderer, so we'll implement the .euiScreenReaderOnly suggestion, thank you.

Q: Are there any issues if the contents of a details cell were wrapped in an EuiFocusTrap, to allow keyboard navigation between the focusable-content in the details cell?

If you are still using the underlying EuiDataGridCell component for the row details "cell", that focus functionality should come for free if 1. you have more than 1 interactive/tabbable child within the cell, and 2. your cell does not have an expansion popover. You can see this behavior in action in our prod docs:

• https://elastic.github.io/eui/#/tabular-content/data-grid-cells-popovers#focus
• Click the leftmost cell with two "Yes | No" buttons
• Press the Enter key to enter the cell focus trap
• Note that the user is now focus trapped & can tab/navigate between items within the cell

perfect, thanks!

[michaelolo24]

Just hopping in here to say thank you so much @cee-chen. We really appreciate this and are excited to be your first guinea pigs in implementing this 🤗