Skip to content
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.

Sign up for GitHub

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

[EuiDataGrid] Redesign grid header #7371

Merged
merged 16 commits into from
Nov 21, 2023
Merged

[EuiDataGrid] Redesign grid header #7371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c0afbe0
Update grid header styles
jughosta Nov 16, 2023
02f5e13
Update snapshots
jughosta Nov 16, 2023
718bfea
Fix styles
jughosta Nov 16, 2023
70ad0aa
Show the icon only on hover
jughosta Nov 16, 2023
cc321d6
Extend tests
jughosta Nov 16, 2023
d6baabe
Add changelog
jughosta Nov 16, 2023
73ad361
Update animations
jughosta Nov 16, 2023
53509b8
Update snapshots
jughosta Nov 16, 2023
ff1cbe1
Fix styles
jughosta Nov 16, 2023
9759255
Update class names
jughosta Nov 16, 2023
7d041bf
Merge branch 'datagrid-redesigns' into 7137-grid-header-updates
cee-chen Nov 21, 2023
596e717
[CSS cleanup] Remove need for extra flex wrapper
cee-chen Nov 21, 2023
003db25
Fix right schemas/columns
cee-chen Nov 21, 2023
5a4b315
Fix non-centered boxes icon
cee-chen Nov 21, 2023
afc7fb1
Remove unnecessary extra className
cee-chen Nov 21, 2023
08c9a47
Copy tweaks
cee-chen Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelogs/upcoming/7371.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- Updated `EuiDataGrid` column header cells to show the sort arrow after the heading text, instead of before
- Updated `EuiDataGrid`'s column header actions icon from a chevron to `boxesVertical`

**Bug fixes**

- Fixed `EuiDataGrid`'s numeric and currency column heading cells to be correctly right-aligned
104 changes: 64 additions & 40 deletions src/components/datagrid/__snapshots__/data_grid.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,12 +617,15 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
>
A
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -667,12 +670,15 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
>
B
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -1049,12 +1055,15 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
>
A
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -1099,12 +1108,15 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
>
B
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -1718,12 +1730,15 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = `
>
Column A
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -1770,12 +1785,15 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = `
More Elements
</div>
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -2130,12 +2148,15 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
>
A
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-A"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -2180,12 +2201,15 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
>
B
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-B"
/>
</div>
</button>
</div>
<p
Expand Down
26 changes: 16 additions & 10 deletions src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,15 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render
>
columnA
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-columnA"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-columnA"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -90,12 +93,15 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render
>
columnB
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-columnB"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-columnB"
/>
</div>
</button>
</div>
<p
Expand Down
26 changes: 16 additions & 10 deletions src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,15 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
>
columnA
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-columnA"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-columnA"
/>
</div>
</button>
</div>
<p
Expand Down Expand Up @@ -94,12 +97,15 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
>
columnB
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-columnB"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-columnB"
/>
</div>
</button>
</div>
<p
Expand Down
13 changes: 8 additions & 5 deletions src/components/datagrid/body/header/__snapshots__/data_grid_header_cell.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ exports[`EuiDataGridHeaderCell renders 1`] = `
>
someColumn
</div>
<span
<div
class="euiDataGridHeaderCell__icon"
color="text"
data-euiicon-type="arrowDown"
data-test-subj="dataGridHeaderCellActionButton-someColumn"
/>
>
<span
color="text"
data-euiicon-type="boxesVertical"
data-test-subj="dataGridHeaderCellActionButton-someColumn"
/>
</div>
</button>
</div>
<p
Expand Down
64 changes: 38 additions & 26 deletions src/components/datagrid/body/header/_data_grid_header_row.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@
align-items: center;
display: flex;

&.euiDataGridHeaderCell--numeric {
text-align: right;
}

&.euiDataGridHeaderCell--currency {
text-align: right;
}

&:focus {
@include euiDataGridCellFocus;
border-top: none;
Expand All @@ -37,38 +29,58 @@
border-top: none;
}

.euiDataGridHeaderCell__sortingArrow {
margin-right: $euiSizeXS;
}

.euiDataGridHeaderCell__button {
flex: 0 0 auto;
position: relative;
align-items: center;
display: flex;
align-items: center;
gap: $euiSizeXS;
width: 100%;
font-weight: $euiFontWeightBold;
outline: none;

.euiDataGridHeaderCell__sortingArrow {
flex-grow: 0;
}
}

.euiDataGridHeaderCell__content {
@include euiTextTruncate;
}

.euiDataGridHeaderCell__sortingArrow {
flex: 0 0 auto; // Ensure icon doesn't shrink
}

.euiDataGridHeaderCell__icon {
flex: 0 0 auto; // Ensure icon doesn't shrink
margin-left: auto; // Aligns the icon to the right
// Center the icon vertically
display: flex;
align-items: center;
height: $euiSize;
overflow: hidden;
white-space: nowrap;
text-align: left;
flex-grow: 1;
align-self: baseline;
width: 0;
opacity: 0;
transition: width $euiAnimSpeedFast ease-in, opacity $euiAnimSpeedSlow ease-in;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreadelrio Can I just check in with you super quick - now that cell actions no longer have the "slide in from the right" behavior (#7343), do we want this icon to still have it? Or do we want to opt for the boxes icon to always be visible?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe @MichaelMarcialis was behind this suggestion. Michael, can we have your input here? From my perspective, I like that the boxes icon appears on hover. However, I do find it a bit odd that when the columns are aligned right, the appearance of the boxes icon causes a shift in the column heading. Because of this, I would suggest we consider either having the boxes icon always visible or overlay it on top of the heading on hover, that way we no longer have a shift in the heading's text.

Screen Recording 2023-11-20 at 8 57 48 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelMarcialis chatted about this in sync this morning and agreed it feels a little odd, but not super odd, so we're going to move ahead with this as-is for now, with the asterisk that we may come back and revisit this once the schema tokens have been added.

(Michael, feel free to give a more complete/accurate summary of your thoughts - I'm sorta rushing to merge this in, apologies!!)

Copy link
Contributor

@MichaelMarcialis MichaelMarcialis Nov 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a bit of background, my original suggestion originates from the Discover design refresh that @andreadelrio and I collaborated on. With our suggestion to prepend field type tokens to the column headers, we thought that only showing the boxesVertical icon when the user is hovering/focusing that column heading helped to cut down on the visual noise and reveal that additional actions could be taken when the user is in a position to use them. For those reasons, I still prefer the idea of hiding the boxesVertical icon until the user hovers/focuses the column headings. As for the right-aligned text shifting, personally I don't mind it with the animation/transition that @jughosta has in place. I think it does a good job of softening its appearance. However, if I'm in the minority here, I'm happy to discuss further.

Hover

That said, @cee-chen recently implemented a newly suggested approach to revealing action buttons when hovering/focusing data grid body cells, as mentioned above. Our approach with the column headings here is technically inconsistent with that new cell body approach. But then again, the data grid column headings have always functioned and appeared differently than the body cells, so perhaps it's a non-issue.

TL;DR: My personal suggestion would be to keep it as @jughosta has here, with the potential future caveat that we may at some point in the future want the action button for data grid column headers to be given the same treatment as body cells. Thoughts?

}

&:focus-within,
&:hover,
.euiPopover-isOpen {
.euiDataGridHeaderCell__icon {
width: $euiSize;
opacity: 1;
}
}
}

// Align numeric and currency schemas to the right
&.euiDataGridHeaderCell--numeric,
&.euiDataGridHeaderCell--currency {
justify-content: flex-end; // Accounts for columns with disabled actions (no popover)

.euiDataGridHeaderCell__button {
justify-content: flex-end; // Accounts for cells with a popover
}

.euiDataGridHeaderCell__icon {
flex-grow: 0;
flex-basis: auto;
width: auto;
padding-left: $euiSizeXS;
margin-left: 0;
}
}
}
Expand Down
Loading