Skip to content

Commit

Permalink
[Single UI polish] Set dynamic minRowHeight based on grid density
Browse files Browse the repository at this point in the history 
Previously rows had a static 34px min height which looked super out of place next to lineCount (which correctly sets row height based on density). This especially looked bad after the align-items baseline change (which was required to get multi-line elements working however).

This basically updates the single/undefined state to match how lineCount work and to dynamically update the row height based on density.

It also increase default row height for normal density to 36px - assuming Amsterdam theme as default. I was previously using '34' as a shorthand for 'default height' in many unit tests, so I've done a general find&replace where it made sense

I also removed the default row height fallback for rowHeightUtils - getCalculatedHeight will pick the defaultHeight (which is minRowHeight) in any case since it will be larger than 0, so there's no need for this to be a realistic number
  • Loading branch information
@cee-chen
cee-chen committed Nov 10, 2021
1 parent 4a1fcbf commit 89b2783
Show file tree
Hide file tree
Showing 11 changed files with 132 additions and 78 deletions.
68 changes: 34 additions & 34 deletions src/components/datagrid/__snapshots__/data_grid.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ Array [
style="position:relative;height:9007199254740991px;width:9007199254740991px;overflow:auto;-webkit-overflow-scrolling:touch;will-change:transform;direction:ltr"
>
<div
style="height:102px;width:200px"
style="height:108px;width:200px"
>
<div
class="euiDataGridHeader"
Expand Down Expand Up @@ -1136,7 +1136,7 @@ Array [
data-gridcell-id="0,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:0px;height:34px;width:100px"
style="position:absolute;left:0;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1164,7 +1164,7 @@ Array [
data-gridcell-id="0,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:0px;height:34px;width:100px"
style="position:absolute;left:100px;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1192,7 +1192,7 @@ Array [
data-gridcell-id="1,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:34px;height:34px;width:100px"
style="position:absolute;left:0;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1220,7 +1220,7 @@ Array [
data-gridcell-id="1,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:34px;height:34px;width:100px"
style="position:absolute;left:100px;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1248,7 +1248,7 @@ Array [
data-gridcell-id="2,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:68px;height:34px;width:100px"
style="position:absolute;left:0;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1276,7 +1276,7 @@ Array [
data-gridcell-id="2,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:68px;height:34px;width:100px"
style="position:absolute;left:100px;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1452,7 +1452,7 @@ Array [
style="position:relative;height:9007199254740991px;width:9007199254740991px;overflow:auto;-webkit-overflow-scrolling:touch;will-change:transform;direction:ltr"
>
<div
style="height:102px;width:300px"
style="height:108px;width:300px"
>
<div
class="euiDataGridHeader"
Expand Down Expand Up @@ -1571,7 +1571,7 @@ Array [
data-gridcell-id="0,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:0px;height:34px;width:50px"
style="position:absolute;left:0;top:0px;height:36px;width:50px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1618,7 +1618,7 @@ Array [
data-gridcell-id="0,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:50px;top:0px;height:34px;width:100px"
style="position:absolute;left:50px;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1646,7 +1646,7 @@ Array [
data-gridcell-id="0,2"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:150px;top:0px;height:34px;width:100px"
style="position:absolute;left:150px;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1674,7 +1674,7 @@ Array [
data-gridcell-id="0,3"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:250px;top:0px;height:34px;width:50px"
style="position:absolute;left:250px;top:0px;height:36px;width:50px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1721,7 +1721,7 @@ Array [
data-gridcell-id="1,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:34px;height:34px;width:50px"
style="position:absolute;left:0;top:36px;height:36px;width:50px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1768,7 +1768,7 @@ Array [
data-gridcell-id="1,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:50px;top:34px;height:34px;width:100px"
style="position:absolute;left:50px;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1796,7 +1796,7 @@ Array [
data-gridcell-id="1,2"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:150px;top:34px;height:34px;width:100px"
style="position:absolute;left:150px;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1824,7 +1824,7 @@ Array [
data-gridcell-id="1,3"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:250px;top:34px;height:34px;width:50px"
style="position:absolute;left:250px;top:36px;height:36px;width:50px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1871,7 +1871,7 @@ Array [
data-gridcell-id="2,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:68px;height:34px;width:50px"
style="position:absolute;left:0;top:72px;height:36px;width:50px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1918,7 +1918,7 @@ Array [
data-gridcell-id="2,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:50px;top:68px;height:34px;width:100px"
style="position:absolute;left:50px;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1946,7 +1946,7 @@ Array [
data-gridcell-id="2,2"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:150px;top:68px;height:34px;width:100px"
style="position:absolute;left:150px;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -1974,7 +1974,7 @@ Array [
data-gridcell-id="2,3"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:250px;top:68px;height:34px;width:50px"
style="position:absolute;left:250px;top:72px;height:36px;width:50px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2166,7 +2166,7 @@ Array [
style="position:relative;height:9007199254740991px;width:9007199254740991px;overflow:auto;-webkit-overflow-scrolling:touch;will-change:transform;direction:ltr"
>
<div
style="height:102px;width:200px"
style="height:108px;width:200px"
>
<div
class="euiDataGridHeader"
Expand Down Expand Up @@ -2257,7 +2257,7 @@ Array [
data-gridcell-id="0,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:0px;height:34px;width:100px"
style="position:absolute;left:0;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2285,7 +2285,7 @@ Array [
data-gridcell-id="0,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:0px;height:34px;width:100px"
style="position:absolute;left:100px;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2313,7 +2313,7 @@ Array [
data-gridcell-id="1,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:34px;height:34px;width:100px"
style="position:absolute;left:0;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2341,7 +2341,7 @@ Array [
data-gridcell-id="1,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:34px;height:34px;width:100px"
style="position:absolute;left:100px;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2369,7 +2369,7 @@ Array [
data-gridcell-id="2,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:68px;height:34px;width:100px"
style="position:absolute;left:0;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2397,7 +2397,7 @@ Array [
data-gridcell-id="2,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:68px;height:34px;width:100px"
style="position:absolute;left:100px;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2570,7 +2570,7 @@ Array [
style="position:relative;height:9007199254740991px;width:9007199254740991px;overflow:auto;-webkit-overflow-scrolling:touch;will-change:transform;direction:ltr"
>
<div
style="height:102px;width:200px"
style="height:108px;width:200px"
>
<div
class="euiDataGridHeader"
Expand Down Expand Up @@ -2659,7 +2659,7 @@ Array [
data-gridcell-id="0,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:0px;height:34px;width:100px"
style="position:absolute;left:0;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2687,7 +2687,7 @@ Array [
data-gridcell-id="0,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:0px;height:34px;width:100px"
style="position:absolute;left:100px;top:0px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2715,7 +2715,7 @@ Array [
data-gridcell-id="1,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:34px;height:34px;width:100px"
style="position:absolute;left:0;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2743,7 +2743,7 @@ Array [
data-gridcell-id="1,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:34px;height:34px;width:100px"
style="position:absolute;left:100px;top:36px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2771,7 +2771,7 @@ Array [
data-gridcell-id="2,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:68px;height:34px;width:100px"
style="position:absolute;left:0;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -2799,7 +2799,7 @@ Array [
data-gridcell-id="2,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:100px;top:68px;height:34px;width:100px"
style="position:absolute;left:100px;top:72px;height:36px;width:100px"
tabindex="-1"
>
<div
Expand Down
6 changes: 3 additions & 3 deletions src/components/datagrid/body/__snapshots__/data_grid_body.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`EuiDataGridBody renders 1`] = `
style="position:relative;height:9007199254740991px;width:9007199254740991px;overflow:auto;-webkit-overflow-scrolling:touch;will-change:transform;direction:ltr"
>
<div
style="height:34px;width:120px"
style="height:36px;width:120px"
>
<div
class="euiDataGridHeader"
Expand Down Expand Up @@ -93,7 +93,7 @@ exports[`EuiDataGridBody renders 1`] = `
data-gridcell-id="0,0"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:0;top:0px;height:34px;width:20px"
style="position:absolute;left:0;top:0px;height:36px;width:20px"
tabindex="-1"
>
<div
Expand Down Expand Up @@ -123,7 +123,7 @@ exports[`EuiDataGridBody renders 1`] = `
data-gridcell-id="0,1"
data-test-subj="dataGridRowCell"
role="gridcell"
style="position:absolute;left:20px;top:0px;height:34px"
style="position:absolute;left:20px;top:0px;height:36px"
tabindex="-1"
>
<div
Expand Down
3 changes: 2 additions & 1 deletion src/components/datagrid/body/data_grid_body.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ describe('EuiDataGridBody', () => {
switchColumnPos: jest.fn(),
schemaDetectors,
rowHeightUtils: mockRowHeightUtils,
minRowHeight: 36,
};

beforeAll(() => {
Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
configurable: true,
value: 34,
value: 36,
});
});

Expand Down
8 changes: 4 additions & 4 deletions src/components/datagrid/body/data_grid_body.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
useMutationObserver,
} from '../../observer/mutation_observer';
import { useResizeObserver } from '../../observer/resize_observer';
import { DEFAULT_ROW_HEIGHT } from '../row_height_utils';
import { EuiDataGridCell } from './data_grid_cell';
import {
DataGridSortingContext,
Expand Down Expand Up @@ -508,14 +507,15 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
[rowHeightUtils]
);

const [minRowHeight, setRowHeight] = useState(DEFAULT_ROW_HEIGHT);
const [minRowHeight, setRowHeight] = useState(props.minRowHeight);

useEffect(() => {
// Reset the minimum row height when switching back to `undefined` rowHeightsOptions (e.g. via the display selector)
// This should also adjust the min height dynamically whenever density changes
if (!rowHeightsOptions) {
setRowHeight(DEFAULT_ROW_HEIGHT);
setRowHeight(props.minRowHeight);
}
}, [rowHeightsOptions]);
}, [rowHeightsOptions, props.minRowHeight]);

const defaultHeight = useMemo(() => {
return rowHeightsOptions?.defaultHeight
Expand Down
Loading

0 comments on commit 89b2783

Please sign in to comment.