[EuiDescriptionList] Add new `columnWidths` prop #7146

cee-chen · 2023-08-30T22:32:34Z

Summary

This PR adds a new prop to EuiDescriptionList to smooth over changes made in #7062 (primarily the change that converted column data grids to using display: grid).

Unfortunately, many Kibana teams and EUI itself were setting custom column widths via CSS flex-basis or width overrides, e.g.:

eui/src/components/datagrid/controls/_data_grid_keyboard_shortcuts.scss

Lines 8 to 16 in a4493d9

    
           .euiDescriptionList { 
        
             .euiDescriptionList__title { 
        
               width: 25%; 
        
             } 
        
             .euiDescriptionList__description { 
        
               width: 75%; 
        
             } 
        
           }

The new display: grid CSS completely ignores both properties, resulting in broken looking UIs:

This new prop adds a quick and easy way for consumers to customize column widths (clearly a frequently desired ability, considering there are 10+ CSS overrides in Kibana alone) without needing to know CSS grid to do so.

⚠️ NOTE: This PR should be released and combined with elastic/kibana#165047, and the upgrade should be subsequently updated to use this new prop and remove as many Kibana CSS overrides of EuiDescriptionList as possible.

QA

Go to https://eui.elastic.co/pr_7146/#/tabular-content/data-grid
Click the keyboard icon in the data grid toolbar (top right)
Confirm the popover description list displays correctly compared to production:
QA/review https://eui.elastic.co/pr_7146/#/display/description-list#as-columns

General checklist

Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs (using @default if default values are missing) and playground toggles
Added documentation
Added or updated jest ~~and cypress tests~~
A changelog entry exists and is marked appropriately

+ newline nit

kibanamachine · 2023-08-30T23:07:36Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_7146/

cee-chen · 2023-08-30T23:26:06Z

src/components/description_list/description_list_types.ts

+   * _Advanced usage note:_ column width strings also accept [CSS grid special units,
+   * sizing, keywords, and sizing functions](https://css-tricks.com/snippets/css/complete-guide-grid/#aa-special-units-functions).
+   */
+  columnWidths?: [number | string, number | string];


Looking for feedback on whether folks think the [x, y] array syntax is the most intuitive for devs, or whether we want to use object notation for more specific keys, e.g.

columnWidths={{ titles: '100px', descriptions: '300px', }}

My 2¢ is the array is the more intuitive syntax. When I think of columns, I'm already thinking in an array-like structure. Being able to set an array and pass a string or number in two places, without having to know the correct key names, is huge. Admittedly this is less of an issue with TypeScript suggestions, but I still lean toward array.

That's an opinion loosely held, so I have no objection if there's a strong argument to move to an object.

My 2¢ is the array is the more intuitive syntax. When I think of columns, I'm already thinking in an array-like structure.

I really like this reasoning - thanks Trevor! I will note one possible reason to use an object instead is if a consumer wants to specify the width of the title column but let the description column remain auto - an obj allows that, the array (currently) doesn't.

That being said, I don't see that as a use case right now, so let's move forward with the array syntax for now. We're looking to move fast due to this affecting the current Kibana upgrade, and if absolutely needed, we can come back in later and add other types with an |.

That's an excellent plan!

1Copenut

One copy suggestion and my 2¢ opinion on object vs. array for setting column widths. After that I'm ready to mark approved. Thanks for putting this together @cee-chen

src-docs/src/views/description_list/description_list_example.js

1Copenut · 2023-08-31T16:52:17Z

src/components/description_list/description_list_types.ts

+   * _Advanced usage note:_ column width strings also accept [CSS grid special units,
+   * sizing, keywords, and sizing functions](https://css-tricks.com/snippets/css/complete-guide-grid/#aa-special-units-functions).
+   */
+  columnWidths?: [number | string, number | string];


My 2¢ is the array is the more intuitive syntax. When I think of columns, I'm already thinking in an array-like structure. Being able to set an array and pass a string or number in two places, without having to know the correct key names, is huge. Admittedly this is less of an issue with TypeScript suggestions, but I still lean toward array.

That's an opinion loosely held, so I have no objection if there's a strong argument to move to an object.

1Copenut

💯 LGTM!

kibanamachine · 2023-08-31T17:43:55Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_7146/

elasticmachine · 2023-08-31T18:01:45Z

💚 Build Succeeded

Buildkite Build
Commit: 1ac6522

History

💚 Build #491 succeeded 15eecf6

## PR change summary `v87.2.0`⏩`v88.1.0` ⚠️ The biggest thing to QA in this PR is several **breaking changes** to `EuiDescriptionList`. ### Description list `columnWidths` prop This PR introduces a new `columnWidths` prop and removes several Kibana instances of custom CSS overrides to title/description column widths. The primary motivation behind this is not just to reduce custom CSS, but also because v88.0.0 introduced an underlying CSS change of `column` description lists to using [`display: grid` CSS](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout). The new prop allows us to support existing description list custom widths while not requiring Kibana developers to understand or write grid CSS (except for 1 or two scenarios around `max-width`). ⚠️ **No user-facing UI around column widths should have regressed as a result of these changes. If they have, please let us know in this PR.** ### Description list gutter size changes The prop `gutterSize` has been renamed to `rowGutterSize` and the default size is now `s` instead of `m`. The change to `s` from `m` means there is an **expected** smaller gap between list items (see below screenshots): **Current `EuiDescriptionList` with default `rowGutterSize="s"`** <img width="753" alt="" src="https://github.com/elastic/kibana/assets/934879/c17aef28-ed3b-40c4-84c3-396e788b13bb"> **Prior `EuiDescriptionList` with default `gutterSize="m"`** <img width="721" alt="" src="https://github.com/elastic/kibana/assets/934879/84d5f5a2-8fa6-4f99-9dc0-73fd143aa1e1"> If Kibana teams prefer to keep the previous `m` gutter for their instances of `EuiDescriptionList`, you have a couple of options: 1. Let EUI team know in the PR and we can set usage back to what it was before 2. Set `rowGutterSize="m"` yourselves manually --- ## [`88.1.0`](https://github.com/elastic/eui/tree/v88.1.0) - Added `font.defaultUnits` theme token. EUI component font sizes default to `rem` units - this token allows consumers to configure this to `px` or `em` ([#7133](elastic/eui#7133)) - Updated `EuiDescriptionList` with new `columnWidths` prop ([#7146](elastic/eui#7146)) **Bug fixes** - Fixed `EuiDataGrid`'s keyboard shortcuts popover display ([#7146](elastic/eui#7146)) **CSS-in-JS conversions** - Renamed `useEuiFontSize()`'s `measurement` option to `unit` for clarity ([#7133](elastic/eui#7133)) ## [`88.0.0`](https://github.com/elastic/eui/tree/v88.0.0) - Updated `EuiDescriptionList` with a new `columnGutterSize` prop ([#7062](elastic/eui#7062)) **Deprecations** - Deprecated `EuiSuggest`. We recommend using `EuiSelectable` or `EuiComboBox` instead ([#7122](elastic/eui#7122)) - Deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([#7122](elastic/eui#7122)) - Deprecated `EuiColorStops`. We recommend copying the component to your application if necessary ([#7122](elastic/eui#7122)) - Deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([#7122](elastic/eui#7122)) **Breaking changes** - Renamed `EuiDescriptionList`'s `gutterSize` prop to `rowGutterSize` ([#7062](elastic/eui#7062)) - `EuiDescriptionList`'s `rowGutterSize` prop now defaults to a size of `s` (was previously `m`) ([#7062](elastic/eui#7062)) **Accessibility** - Fixed the dark mode colors of inline `EuiDescriptionListTitle`s to meet WCAG color contrast requirements ([#7062](elastic/eui#7062)) **CSS-in-JS conversions** - Converted `EuiKeyPadMenuItem` to Emotion; Removed `$euiKeyPadMenuSize` and `$euiKeyPadMenuMarginSize` ([#7118](elastic/eui#7118)) --------- Co-authored-by: Cee Chen <[email protected]> Co-authored-by: Cee Chen <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Nikita Indik <[email protected]>

## PR change summary `v87.2.0`⏩`v88.1.0` ⚠️ The biggest thing to QA in this PR is several **breaking changes** to `EuiDescriptionList`. ### Description list `columnWidths` prop This PR introduces a new `columnWidths` prop and removes several Kibana instances of custom CSS overrides to title/description column widths. The primary motivation behind this is not just to reduce custom CSS, but also because v88.0.0 introduced an underlying CSS change of `column` description lists to using [`display: grid` CSS](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout). The new prop allows us to support existing description list custom widths while not requiring Kibana developers to understand or write grid CSS (except for 1 or two scenarios around `max-width`). ⚠️ **No user-facing UI around column widths should have regressed as a result of these changes. If they have, please let us know in this PR.** ### Description list gutter size changes The prop `gutterSize` has been renamed to `rowGutterSize` and the default size is now `s` instead of `m`. The change to `s` from `m` means there is an **expected** smaller gap between list items (see below screenshots): **Current `EuiDescriptionList` with default `rowGutterSize="s"`** <img width="753" alt="" src="https://github.com/elastic/kibana/assets/934879/c17aef28-ed3b-40c4-84c3-396e788b13bb"> **Prior `EuiDescriptionList` with default `gutterSize="m"`** <img width="721" alt="" src="https://github.com/elastic/kibana/assets/934879/84d5f5a2-8fa6-4f99-9dc0-73fd143aa1e1"> If Kibana teams prefer to keep the previous `m` gutter for their instances of `EuiDescriptionList`, you have a couple of options: 1. Let EUI team know in the PR and we can set usage back to what it was before 2. Set `rowGutterSize="m"` yourselves manually --- ## [`88.1.0`](https://github.com/elastic/eui/tree/v88.1.0) - Added `font.defaultUnits` theme token. EUI component font sizes default to `rem` units - this token allows consumers to configure this to `px` or `em` ([elastic#7133](elastic/eui#7133)) - Updated `EuiDescriptionList` with new `columnWidths` prop ([elastic#7146](elastic/eui#7146)) **Bug fixes** - Fixed `EuiDataGrid`'s keyboard shortcuts popover display ([elastic#7146](elastic/eui#7146)) **CSS-in-JS conversions** - Renamed `useEuiFontSize()`'s `measurement` option to `unit` for clarity ([elastic#7133](elastic/eui#7133)) ## [`88.0.0`](https://github.com/elastic/eui/tree/v88.0.0) - Updated `EuiDescriptionList` with a new `columnGutterSize` prop ([elastic#7062](elastic/eui#7062)) **Deprecations** - Deprecated `EuiSuggest`. We recommend using `EuiSelectable` or `EuiComboBox` instead ([elastic#7122](elastic/eui#7122)) - Deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([elastic#7122](elastic/eui#7122)) - Deprecated `EuiColorStops`. We recommend copying the component to your application if necessary ([elastic#7122](elastic/eui#7122)) - Deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([elastic#7122](elastic/eui#7122)) **Breaking changes** - Renamed `EuiDescriptionList`'s `gutterSize` prop to `rowGutterSize` ([elastic#7062](elastic/eui#7062)) - `EuiDescriptionList`'s `rowGutterSize` prop now defaults to a size of `s` (was previously `m`) ([elastic#7062](elastic/eui#7062)) **Accessibility** - Fixed the dark mode colors of inline `EuiDescriptionListTitle`s to meet WCAG color contrast requirements ([elastic#7062](elastic/eui#7062)) **CSS-in-JS conversions** - Converted `EuiKeyPadMenuItem` to Emotion; Removed `$euiKeyPadMenuSize` and `$euiKeyPadMenuMarginSize` ([elastic#7118](elastic/eui#7118)) --------- Co-authored-by: Cee Chen <[email protected]> Co-authored-by: Cee Chen <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Nikita Indik <[email protected]>

cee-chen added 7 commits August 30, 2023 15:41

[EuiDescriptionList] Add new columnWidths prop

28e792d

[misc tests cleanup] rename test blocks to their exact prop names

d95e174

+ newline nit

[docs] Add new columnWidths demo/example

63bba81

[misc docs cleanup] remove unnecessary fragments

4ffabaf

[EuiDataGrid] Fix keyboard shortcuts descritpion list

1598ebd

[EuiDataGrid] Fix misc incorrect docs prop name

b5645c0

changelog

15eecf6

cee-chen force-pushed the description-list-column-widths branch from 8910b7c to 15eecf6 Compare August 30, 2023 22:41

cee-chen requested review from a team and 1Copenut August 30, 2023 23:23

cee-chen marked this pull request as ready for review August 30, 2023 23:24

cee-chen commented Aug 30, 2023

View reviewed changes

1Copenut reviewed Aug 31, 2023

View reviewed changes

[PR feedback] docs copy

1ac6522

1Copenut approved these changes Aug 31, 2023

View reviewed changes

cee-chen enabled auto-merge (squash) August 31, 2023 17:36

cee-chen merged commit 3aea311 into elastic:main Aug 31, 2023
7 checks passed

cee-chen deleted the description-list-column-widths branch August 31, 2023 18:01

cee-chen mentioned this pull request Aug 30, 2023

Upgrade EUI to v88.1.0 elastic/kibana#165047

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiDescriptionList] Add new `columnWidths` prop #7146

[EuiDescriptionList] Add new `columnWidths` prop #7146

cee-chen commented Aug 30, 2023 •

edited

Loading

kibanamachine commented Aug 30, 2023

cee-chen Aug 30, 2023

1Copenut Aug 31, 2023

cee-chen Aug 31, 2023

1Copenut Aug 31, 2023

1Copenut left a comment

1Copenut Aug 31, 2023

1Copenut left a comment

kibanamachine commented Aug 31, 2023

elasticmachine commented Aug 31, 2023

	.euiDescriptionList {
	.euiDescriptionList__title {
	width: 25%;
	}

	.euiDescriptionList__description {
	width: 75%;
	}
	}

[EuiDescriptionList] Add new columnWidths prop #7146

[EuiDescriptionList] Add new columnWidths prop #7146

Conversation

cee-chen commented Aug 30, 2023 • edited Loading

Summary

QA

General checklist

kibanamachine commented Aug 30, 2023

cee-chen Aug 30, 2023

Choose a reason for hiding this comment

1Copenut Aug 31, 2023

Choose a reason for hiding this comment

cee-chen Aug 31, 2023

Choose a reason for hiding this comment

1Copenut Aug 31, 2023

Choose a reason for hiding this comment

1Copenut left a comment

Choose a reason for hiding this comment

1Copenut Aug 31, 2023

Choose a reason for hiding this comment

1Copenut left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 31, 2023

elasticmachine commented Aug 31, 2023

💚 Build Succeeded

History

[EuiDescriptionList] Add new `columnWidths` prop #7146

[EuiDescriptionList] Add new `columnWidths` prop #7146

cee-chen commented Aug 30, 2023 •

edited

Loading