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.

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

[High Contrast Mode] Tables and data grid #8226

Merged
merged 8 commits into from
Dec 12, 2024
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Non blocking, likely unrelated (just attaching it here):
I noticed that the mobile actions seem to be placed differently between color modes.
It seems like the inset-block-start doesn't always apply. Maybe we could change to margin-block-start? 🤔

storybook

Screen.Recording.2024-12-12.at.14.45.00.mov

EUI docs

Screen.Recording.2024-12-12.at.14.52.51.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha what the heck

Copy link
Contributor Author

@cee-chen cee-chen Dec 12, 2024

Choose a reason for hiding this comment

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

I think this is a bizarre Storybook/rendering thing. If I start the story in light mode, the opposite effect happens in dark mode. And if you refresh the page, it fixes itself again. I'm going to leave it alone for now to not break production styles 🤷 (couldn't get top: 0/margin-top to work either)

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, weird indeed. I was able to reproduce it in EUI docs as well. 🫤
I'm ok not touching it here, as it's not related to the changes done here. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh I missed that in the second screencap, apologies. I can repro it in production EUI docs as well. With that in mind (since it's not caused by high contrast mode), can we file a separate issue for this bug? It's honestly bonkers haha, I don't think I've ever seen anything like it. I wonder if it's something to do with tables being tables 🫠

Copy link
Contributor

Choose a reason for hiding this comment

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

🔗 I added an issue for it.

Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,15 @@ export const ExpandedNestedTable: Story = {
},
};

export const HighContrastMobile: Story = {
tags: ['vrt-only'],
globals: { highContrastMode: true },
args: {
...ExpandedRow.args,
responsiveBreakpoint: true,
},
};

const StatefulPlayground = ({
items,
pagination,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
logicalSizeCSS,
mathWithUnits,
} from '../../../../global_styling';
import { highContrastModeStyles } from '../../../../global_styling/functions/high_contrast';

import { euiDataGridVariables } from '../../data_grid.styles';
import {
Expand Down Expand Up @@ -126,6 +127,11 @@ export const euiDataGridCellActionsStyles = (euiThemeContext: UseEuiTheme) => {
${logicalCSS('width', '120%')}
${logicalCSS('height', '100%')}
}

/* Remove button borders in high contrast mode */
${highContrastModeStyles(euiThemeContext, {
preferred: 'border: none;',
})}
`,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ export const euiDataGridColumnSelectorStyles = (
padding: ${euiTheme.size.xs};

&.euiDataGridColumnSelector__item-isDragging {
${euiShadowLarge(euiThemeContext)}
${euiShadowLarge(euiThemeContext, {
borderAllInHighContrastMode: true,
})}
background-color: ${euiTheme.colors.emptyShade};
}
`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ export const euiDataGridColumnSortingStyles = (
${logicalCSS('padding-horizontal', euiTheme.size.s)}

&.euiDataGridColumnSorting__item-isDragging {
${euiShadowLarge(euiThemeContext)}
${euiShadowLarge(euiThemeContext, {
borderAllInHighContrastMode: true,
})}
background-color: ${euiTheme.colors.emptyShade};
}
`,
Expand Down
23 changes: 17 additions & 6 deletions packages/eui/src/components/datagrid/data_grid.styles.ts
mgadewoll marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
logicalSizeCSS,
mathWithUnits,
} from '../../global_styling';
import { highContrastModeStyles } from '../../global_styling/functions/high_contrast';

export const euiDataGridVariables = (euiThemeContext: UseEuiTheme) => {
const { euiTheme } = euiThemeContext;
Expand Down Expand Up @@ -79,14 +80,24 @@ export const euiDataGridStyles = (euiThemeContext: UseEuiTheme) => {
padding: ${cellPadding[size]};
}

/* Workaround to trim line-clamp and padding - @see https://github.com/elastic/eui/issues/7780 */
.euiDataGridRowCell__content--lineCountHeight,
.euiDataGridRowCell__content--autoBelowLineCountHeight {
${logicalCSS('padding-bottom', 0)}
${logicalCSS(
'border-bottom',
`${cellPadding[size]} solid transparent`
)}
${highContrastModeStyles(euiThemeContext, {
// Workaround to trim line-clamp and padding - @see https://github.com/elastic/eui/issues/7780
none: `
${logicalCSS('padding-bottom', 0)}
${logicalCSS(
'border-bottom',
`${cellPadding[size]} solid transparent`
)}
`,
// Unfortunately this workaround backfires in Windows high contrast themes, which force
// border-color, so we can't use it. We'll use a clip-path workaround instead.
get forced() {
const bottomY = `calc(100% - ${cellPadding[size]})`;
return `clip-path: polygon(0 0, 100% 0, 100% ${bottomY}, 0 ${bottomY});`;
},
})}
}

/* Ensure the column actions button maintains its size for accessible click/tap targeting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,22 @@ export const euiDataGridPaginationStyles = ({ euiTheme }: UseEuiTheme) => ({
${logicalCSS('padding-top', euiTheme.size.xs)}

.euiDataGrid--fullScreen & {
position: relative;
${logicalCSS('padding-bottom', euiTheme.size.xs)}
background-color: ${euiTheme.colors.lightestShade};

/* Use box-shadow instead of border-top to avoid duplicating the border-bottom on grid cells */
box-shadow: ${euiTheme.border.width.thin} 0 0
${euiTheme.border.width.thin} ${euiTheme.border.color};
/* Use a pseudo element instead of:
* 1. border-top directly on the element, to avoid duplicating the border-bottom on grid cells
* 2. box-shadow, so that the border renders on Windows high contrast themes
*/
&::before {
content: '';
position: absolute;
${logicalCSS('top', `-${euiTheme.border.width.thin}`)}
${logicalCSS('horizontal', 0)}
${logicalCSS('border-top', euiTheme.border.thin)}
pointer-events: none;
}
}
`,
});
43 changes: 35 additions & 8 deletions packages/eui/src/components/datagrid/utils/scrolling.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { css } from '@emotion/react';

import { UseEuiTheme } from '../../../services';
import { logicalCSS, mathWithUnits } from '../../../global_styling';
import { highContrastModeStyles } from '../../../global_styling/functions/high_contrast';

export const euiDataGridScrollBarStyles = (euiThemeContext: UseEuiTheme) => {
const { euiTheme } = euiThemeContext;
Expand All @@ -29,26 +30,52 @@ export const euiDataGridScrollBarStyles = (euiThemeContext: UseEuiTheme) => {
/* Ensure the underlying grid is still interactable */
pointer-events: none;

/* Ensure the scrolling data grid body always has border edges regardless of cell position */
box-shadow: inset 0 0 0 ${borderWidth} ${borderColor};
${highContrastModeStyles(euiThemeContext, {
// Ensure the scrolling data grid body always has border edges regardless of cell position
none: `box-shadow: inset 0 0 0 ${borderWidth} ${borderColor};`,
// Windows high contrast themes ignore box-shadow, so use a pseudo element workaround
forced: `
&::before {
content: '';
position: absolute;
inset: 0;
${logicalCSS('border-vertical', euiTheme.border.thin)}
}
&::after {
content: '';
position: absolute;
inset: 0;
${logicalCSS('border-horizontal', euiTheme.border.thin)}
}
`,
})}

.euiDataGrid--bordersHorizontal & {
box-shadow: inset 0 -${mathWithUnits(borderWidth, (x) => x * 2)} 0 -${borderWidth}
${borderColor};
${highContrastModeStyles(euiThemeContext, {
none: `
box-shadow: inset 0 -${mathWithUnits(
borderWidth,
(x) => x * 2
)} 0 -${borderWidth} ${borderColor};
`,
forced: `
&::after {
display: none;
}
`,
})}
}
`,
// Ensure the horizontal scrollbar has a top border
euiDataGrid__scrollBarOverlayBottom: css`
position: absolute;
inset-inline: 0;
${logicalCSS('height', borderWidth)}
background-color: ${borderColor};
${logicalCSS('border-top', euiTheme.border.thin)}
`,
// Ensure the vertical scrollbar has a left border
euiDataGrid__scrollBarOverlayRight: css`
position: absolute;
${logicalCSS('width', borderWidth)}
background-color: ${borderColor};
${logicalCSS('border-left', euiTheme.border.thin)}
`,
// Note: Scroll bar border positions are set via JS inline style, since
// JS has access to the exact OS scrollbar width/height and CSS doesn't
Expand Down
23 changes: 19 additions & 4 deletions packages/eui/src/components/table/table_row.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
euiCanAnimate,
euiBackgroundColor,
logicalCSS,
mathWithUnits,
} from '../../global_styling';
import { euiShadow } from '../../themes/amsterdam/global_styling/mixins';

Expand Down Expand Up @@ -79,9 +80,16 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
${logicalCSS('margin-bottom', cellContentPadding)}

/* EuiPanel styling */
${euiShadow(euiThemeContext, 's')}
${euiShadow(euiThemeContext, 's', {
borderAllInHighContrastMode: true,
})}
background-color: ${euiBackgroundColor(euiThemeContext, 'plain')};
border-radius: ${euiTheme.border.radius.medium};

&:has(+ .euiTableRow-isExpandedRow) {
${logicalCSS('border-bottom-left-radius', 0)}
${logicalCSS('border-bottom-right-radius', 0)}
}
`,
selected: css`
&,
Expand All @@ -108,15 +116,22 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
position: absolute;
${logicalCSS('vertical', 0)}
${logicalCSS('right', mobileSizes.actions.width)}
${logicalCSS('width', euiTheme.border.width.thin)}
background-color: ${euiTheme.border.color};
${logicalCSS('border-right', euiTheme.border.thin)}
}
`,
/**
* Bottom of card - expanded rows
*/
expanded: css`
${logicalCSS('margin-top', `-${mobileSizes.actions.offset}`)}
${logicalCSS(
// On mobile, visually move the expanded row to join up with the
// preceding table row via negative margins
'margin-top',
mathWithUnits(
[cellContentPadding, euiTheme.border.width.thin],
(x, y) => (x + y) * -1
)
)}
/* Padding accounting for the checkbox is already applied via the content */
${logicalCSS('padding-left', cellContentPadding)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ export const euiTableRowCellStyles = (euiThemeContext: UseEuiTheme) => {
content: '';
position: absolute;
${logicalCSS('horizontal', 0)}
${logicalCSS('height', euiTheme.border.width.thin)}
background-color: ${euiTheme.border.color};
${logicalCSS('border-top', euiTheme.border.thin)}
}

/* Minor vertical alignment of cell content */
Expand Down
Loading