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

[High Contrast Mode] Tables and data grid #8226

Merged
merged 8 commits into from
Dec 12, 2024
Merged

[High Contrast Mode] Tables and data grid #8226

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4b347e5
[VRT] Update all tables screenshots
cee-chen Dec 12, 2024
edf83a2
[EuiBasicTable][cleanup] Better align expanded rows and custom action…
cee-chen Dec 12, 2024
8c9c18d
[EuiBasicTable] Fix border rendering in high contrast modes
cee-chen Dec 12, 2024
86f1443
[EuiDataGrid] VRT updates
cee-chen Dec 12, 2024
c758f36
[EuiDataGrid] Improve high contrast column dragging
cee-chen Dec 12, 2024
22c53cf
[EuiDataGrid] Remove borders around cell actions in forced colors mode
cee-chen Dec 12, 2024
12c0e62
[EuiDataGrid] Workaround for `lineCount` text overflow and Windows hi…
cee-chen Dec 12, 2024
7b0d477
[EuiDataGrid] Windows high contrast mode box-shadow workarounds
cee-chen Dec 12, 2024
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
Binary file modified ...reference/chrome_mobile_Tabular_Content_EuiBasicTable_Expanded_Nested_Table.png
View file
Open in desktop
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 •
edited
Loading

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.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...ui/.loki/reference/chrome_mobile_Tabular_Content_EuiBasicTable_Expanded_Row.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 15 additions & 1 deletion packages/eui/src/components/table/table_row.styles.ts
View file
Open in desktop
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 @@ -82,6 +83,11 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
${euiShadow(euiThemeContext, 's')}
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 Down Expand Up @@ -116,7 +122,15 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
* 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