-
Notifications
You must be signed in to change notification settings - Fork 860
[EuiDataGrid] Support rendering the header optionally #9281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
33a1a55 to
92feee7
Compare
- the getRow method may return undefined
92feee7 to
be49e96
Compare
💚 Build SucceededHistory
cc @mgadewoll |
💚 Build Succeeded
History
cc @mgadewoll |
| /* hack to prevent visible flickering on the single initial render when rows are created. | ||
| opacity > 0 ensure the element is anyway present in the DOM */ | ||
| .euiDataGridRowCell--isMounting:where( | ||
| :not(.euiDataGridHeaderCell, .euiDataGridFooterCell) | ||
| ) { | ||
| opacity: 0.01; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't reproduce any flickering on the initial render. Could you provide more details how to test this?
If the change is not related to the conditional rendering of the header in any way (e.g. it doesn't make the flickering issue significantly worse), I'd extract it to a separate PR with an appropriate description, steps to reproduce, pointers what to test for and a link to related issues (if they exist).
| const wrapperDimensions = useResizeObserver(wrapperRef.current); | ||
| const outerGridRef = useRef<HTMLDivElement | null>(null); // container that becomes scrollable | ||
| const innerGridRef = useRef<HTMLDivElement | null>(null); // container sized to fit all content | ||
| const innerGridRef = useRef<HTMLDivElement | null>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this comment removed?
weronikaolejniczak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The showHeader code changes look good 👍🏻 I shared some doubts in separate comments.
Testing notes
Header and toolbar visibility (tested here)
Toolbar and header display as expected. gridStyle.border is default here, so "all" for all 4 cases.
✅ default headerVisiblity={true} & default toolbarVisibility
| HCM off | HCM on | |
|---|---|---|
| Light mode | ![]() |
![]() |
| Dark mode | ![]() |
![]() |
✅ default headerVisiblity={true} & toolbarVisibility={false}
| HCM off | HCM on | |
|---|---|---|
| Light mode | ![]() |
![]() |
| Dark mode | ![]() |
![]() |
✅ headerVisiblity={false} & toolbarVisibility={false}
| HCM off | HCM on | |
|---|---|---|
| Light mode | ![]() |
![]() |
| Dark mode | ![]() |
![]() |
✅ headerVisiblity={false} & default toolbarVisibility
| HCM off | HCM on | |
|---|---|---|
| Light mode | ![]() |
![]() |
| Dark mode | ![]() |
![]() |
Different gridStyle, focusing on border (docs, tested here)
There are no missing or doubled borders. Tested in light mode with HCM off.
✅ horizontal
default headerVisibility={true} |
headerVisibility={false} |
|
|---|---|---|
default toolbarVisibility |
![]() |
![]() |
toolbarVisibility={false} |
![]() |
![]() |
✅ none
default headerVisibility={true} |
headerVisibility={false} |
|
|---|---|---|
default toolbarVisibility |
![]() |
![]() |
toolbarVisibility={false} |
![]() |
![]() |
Screen reader output (tested here)
Because we enrich each cell with screen-reader-only text, not rendering the header doesn't have detrimental effect to a11y. Actually, it yields better screen reader announcements because they are not polluted by the column cell announcements (e.g. "Press the Enter key to view this column's actions" when the Enter key is already mapping to "expanding the cell"). In the JAWS case, the column header is repeated if the header is present.
💡 It could be a nice opportunity to think about:
Is there an easy way we could include this change?
✅ (MacOS) VoiceOver + MacOS
| With header | Without header |
|---|---|
Kapture.2025-12-31.at.12.11.57.mp4 |
Kapture.2025-12-31.at.12.09.42.mp4 |
💭 The only difference seems to be that if the header is present, the number of columns and rows is read out. If it's not present, that data is omitted. And we do pass aria-rowcount directly. Not sure why that is.
✅ (Win) NVDA + Firefox
| With header | Without header |
|---|---|
Kapture.2025-12-31.at.12.26.23.mp4 |
Kapture.2025-12-31.at.12.23.20.mp4 |
💭 (out of scope) The NVDA + data grid experience is very confusing due to browse mode. Only once I browse to a specific cell with arrow down and tab, can I then navigate with the arrow keys. Do you know why that is and if it's something we can improve?
✅ (Win) JAWS + Chrome
| 🔈 With header | 🔈 Without header |
|---|---|
Kapture.2025-12-31.at.12.35.00.mp4 |
Kapture.2025-12-31.at.12.32.23.mp4 |
Potential prod regression
Based on code changes, I tested these potential regression spots:
✅ there are no missing or double borders
✅ on initial render, the data grid displays correctly both with and without the header
✅ when dynamically toggling headerVisibility, the scroll offset is applied correctly and there is no layout shift or jumpiness
✅ there are no renderCustomGridBody usages that access headerRow property without checking for null (no such cases in Kibana)
I couldn't verify this point because I cannot reproduce the issue, even with simulated CPU throttling:
❓ verified the isMounting logic prevents flickering and doesn't leave cells in an invisible state
Personally, if it's not caused or significantly worsened by the changes on the PR, I'd move it to a separate PR with clear steps to reproduce.
























Summary
Closes https://github.com/elastic/eui-private/issues/475
Closes #9000
This PR updates
EuiDataGridto add support for a conditionally rendered header element via a new prop:headerVisibility. The naming is chosen to be in line with the existingtoolbarVisibilityfor the external facing API.headerVisibilitybooleantrueAccessibility
Generally not rendering the header element is not adviced for semantic tables, as its required for providing contextual information to screen readers.
The current implementation of
EuiDataGridadds the contextual information as screen reader only text to each cell which ensures that accessibility is kept as is.Screen.Recording.2025-12-18.at.09.09.155.mov
Why are we making this change?
✨ Feature request: Serving a Kibana feature request.
Screenshots #
Screen.Recording.2025-12-18.at.15.07.11.mov
Usage with
headerVisibility=false+toolbarVisibility=falseImpact to users
🟢 No updates required on consumer side. This is a new, opt-in feature.
QA
headerVisibilityonEuiDataGridand verify it works as expected (ensure to test in different combinations: withtoolbarvisibilitywith differentborderstyles etcGeneral checklist
@defaultif default values are missing) and playground toggles