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

[EuiDescriptionList] Update display to grid for vertical column option with new props #7062

Merged
merged 56 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
58ab7cc
add display grid
kyrspl Aug 9, 2023
1fd6bbd
add eui variables for column gap
kyrspl Aug 9, 2023
040cf19
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 9, 2023
9d46054
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 10, 2023
28ef754
add total max width
kyrspl Aug 10, 2023
d64db92
Update src/components/description_list/description_list_types.ts
kyrspl Aug 11, 2023
c53c8a8
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 11, 2023
9bd8385
update test for column_gap
kyrspl Aug 11, 2023
94c5f68
add changelog
kyrspl Aug 11, 2023
672f564
update documentation
kyrspl Aug 11, 2023
0ee59c5
fix typo
kyrspl Aug 11, 2023
b25bc8f
fix typo in test
kyrspl Aug 11, 2023
64e192c
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 12, 2023
c0c9e1f
update test desc
kyrspl Aug 14, 2023
3896845
update keyboard snapshot
kyrspl Aug 14, 2023
f160fc9
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 14, 2023
8f2c8ef
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 14, 2023
fd1e4f7
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 15, 2023
79cc572
fix typos
kyrspl Aug 15, 2023
8a59812
Update src-docs/src/views/description_list/description_list_example.js
kyrspl Aug 15, 2023
db9e57a
Update src-docs/src/views/description_list/description_list_example.js
kyrspl Aug 15, 2023
dde01e8
[PR feedback] Conditionally render CSS that only applies to `display:…
cee-chen Aug 15, 2023
f8faa78
[PR feedback] Remove unnecessary mobile breakpoint CSS
cee-chen Aug 15, 2023
c11b79f
[PR feedback] Remove `row-gap` CSS
cee-chen Aug 15, 2023
2bd11ee
[PR feedback] Remove `columnGap` from context
cee-chen Aug 15, 2023
d35e84e
prettier lint fix
cee-chen Aug 15, 2023
1028d81
Update downstream snapshots
cee-chen Aug 15, 2023
2c7e070
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 16, 2023
507139a
refactor final
kyrspl Aug 21, 2023
fc12fe6
update context
kyrspl Aug 21, 2023
4ec06df
update tests
kyrspl Aug 21, 2023
a38d4c4
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 21, 2023
d3d8416
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 22, 2023
56a791d
update tests
kyrspl Aug 22, 2023
5311a63
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 22, 2023
521b4d7
Fix lint complaint
cee-chen Aug 22, 2023
a78ec7a
CSS cleanup
cee-chen Aug 22, 2023
3e1ad21
[REFACTOR] Remove `responsiveColumn` CSS entirely, and instead use JS…
cee-chen Aug 22, 2023
0f462d5
[REFACTOR] Remove `responsiveColumns` CSS & logic from child components
cee-chen Aug 22, 2023
7388ea2
[refactor] update child types and tests
cee-chen Aug 22, 2023
0624a3c
misc newline nits
cee-chen Aug 22, 2023
95641d0
Fix row/column gap tests
cee-chen Aug 22, 2023
06ba99a
[BREAKING CHANGE] Prop renames
cee-chen Aug 22, 2023
3e4d1f1
Update datagrid usage
cee-chen Aug 22, 2023
e4669e5
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 23, 2023
c1f74d3
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 23, 2023
5710c75
Update src/components/description_list/description_list_title.styles.ts
kyrspl Aug 23, 2023
2b6a0cb
Update src/components/description_list/description_list_title.styles.ts
kyrspl Aug 23, 2023
3f49cad
Update upcoming_changelogs/7062.md
kyrspl Aug 23, 2023
eae95f4
[docs] Fix prop names
cee-chen Aug 23, 2023
024eb73
Update src/components/description_list/description_list_title.styles.ts
kyrspl Aug 24, 2023
4808e81
removed max-inline-size
kyrspl Aug 24, 2023
0f4bed3
changelog
cee-chen Aug 25, 2023
66354b7
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 25, 2023
a1ffc4b
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 28, 2023
a7a5f7c
Merge branch 'main' into descriptionList_update_vertical
kyrspl Aug 28, 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
35 changes: 30 additions & 5 deletions src/components/description_list/description_list.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,21 @@
*/

import { css } from '@emotion/react';
import { logicalTextAlignCSS, euiMinBreakpoint } from '../../global_styling';
import {
logicalTextAlignCSS,
euiMaxBreakpoint,
euiMinBreakpoint,
} from '../../global_styling';
import { UseEuiTheme } from '../../services';

export const euiDescriptionListStyles = (euiThemeContext: UseEuiTheme) => {
// Flex display for column and responsive column
const TITLE_MAX_WIDTH = '380px';
kyrspl marked this conversation as resolved.
Show resolved Hide resolved

// Grid display for column and responsive column
const columnDisplay = `
display: flex;
display: grid;
grid-template-columns: fit-content(${TITLE_MAX_WIDTH}) 1fr;
align-items: baseline;
flex-wrap: wrap;
`;

return {
Expand All @@ -29,11 +35,30 @@ export const euiDescriptionListStyles = (euiThemeContext: UseEuiTheme) => {
`,
// Responsive columns behave as a row on breakpoints xs-s
responsiveColumn: css`
${euiMaxBreakpoint(euiThemeContext, 'm')} {
grid-template-columns: 1fr;
}
${euiMinBreakpoint(euiThemeContext, 'm')} {
${columnDisplay}
}
`,

// Handles row-gap according to row gutter size
rowGap: {
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
s: css`
row-gap: ${euiThemeContext.euiTheme.size.s};
`,
m: css`
row-gap: ${euiThemeContext.euiTheme.size.m};
`,
},
columnGap: {
s: css`
column-gap: ${euiThemeContext.euiTheme.size.s};
`,
m: css`
column-gap: ${euiThemeContext.euiTheme.size.xl};
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
`,
},
// Alignment
center: css`
${logicalTextAlignCSS('center')}
Expand Down
13 changes: 10 additions & 3 deletions src/components/description_list/description_list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,20 @@ export const EuiDescriptionList: FunctionComponent<
textStyle = 'normal',
titleProps,
type = 'row',
gutterSize = 'm',
gutterSize = 's',
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I just noticed this - changing a default prop that affects end users / how components visually display would be considered a breaking change. We'll need to add a changelog item that notes that EuiDescriptionList now defaults to a gutter size of s.

Copy link
Member

Choose a reason for hiding this comment

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

On the subject of breaking changes - the new max-width applied to the title would also be considered a visually breaking change, hence why I think I'd lean towards making that a new customizable prop as well.

Copy link
Contributor Author

@kyrspl kyrspl Aug 21, 2023

Choose a reason for hiding this comment

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

I think I'd lean towards making that a new customizable prop as well.

I'd rather wait to see whether teams request this. I would want to avoid having situations where a max-inline-size gets abused.

columnGap = 's',
...rest
}) => {
const euiTheme = useEuiTheme();
const styles = euiDescriptionListStyles(euiTheme);

const cssStyles = [styles.euiDescriptionList, styles[type], styles[align]];
const cssStyles = [
styles.euiDescriptionList,
styles[type],
styles.rowGap[gutterSize],
styles.columnGap[columnGap],
styles[align],
];

const classes = classNames('euiDescriptionList', className);

Expand All @@ -66,7 +73,7 @@ export const EuiDescriptionList: FunctionComponent<

return (
<EuiDescriptionListContext.Provider
value={{ type, compressed, textStyle, align, gutterSize }}
value={{ type, compressed, textStyle, align, gutterSize, columnGap }}
>
<dl className={classes} css={cssStyles} {...rest} data-type={type}>
{childrenOrListItems}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ import { EuiDescriptionListProps } from './description_list_types';

type EuiDescriptionListContextValues = Required<
Pick<EuiDescriptionListProps, 'type' | 'textStyle' | 'align' | 'gutterSize'>
> & { compressed?: EuiDescriptionListProps['compressed'] };
> & {
compressed?: EuiDescriptionListProps['compressed'];
columnGap?: EuiDescriptionListProps['columnGap'];
};

export const contextDefaults: EuiDescriptionListContextValues = {
type: 'row',
textStyle: 'normal',
align: 'left',
gutterSize: 'm',
columnGap: 'm',
};

export const EuiDescriptionListContext =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ export const euiDescriptionListDescriptionStyles = (
const { euiTheme } = euiThemeContext;

const columnDisplay = `
${logicalCSS('width', '50%')}
${logicalCSS('padding-left', euiTheme.size.s)}

`;
kyrspl marked this conversation as resolved.
Show resolved Hide resolved

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import {
euiFontSize,
euiTextBreakWord,
logicalTextAlignCSS,
euiMaxBreakpoint,
euiMinBreakpoint,
logicalCSS,
} from '../../global_styling';
import { tint, UseEuiTheme } from '../../services';
Expand All @@ -22,8 +20,7 @@ export const euiDescriptionListTitleStyles = (euiThemeContext: UseEuiTheme) => {
const { euiTheme, colorMode } = euiThemeContext;

const columnDisplay = `
${logicalCSS('width', '50%')}
${logicalCSS('padding-right', euiTheme.size.s)}

`;
return {
euiDescriptionList__title: css`
Expand All @@ -35,15 +32,7 @@ export const euiDescriptionListTitleStyles = (euiThemeContext: UseEuiTheme) => {
column: css`
${columnDisplay}
`,
responsiveColumn: css`
${euiMaxBreakpoint(euiThemeContext, 'm')} {
${logicalCSS('width', '100%')}
padding: 0;
}
${euiMinBreakpoint(euiThemeContext, 'm')} {
${columnDisplay}
}
`,
responsiveColumn: css``,
kyrspl marked this conversation as resolved.
Show resolved Hide resolved
inline: css`
display: inline;
border-radius: ${euiTheme.border.radius.small};
Expand Down
7 changes: 7 additions & 0 deletions src/components/description_list/description_list_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ export interface EuiDescriptionListProps {
* Vertical spacing added between `EuiDescriptionList` elements
*/
gutterSize?: EuiDescriptionListGutterSizes;
/**
* Horizontal spacing added between `EuiDescriptionList` elements
kyrspl marked this conversation as resolved.
Show resolved Hide resolved
*/
columnGap?: EuiDescriptionListColumnGapSizes;
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
}

export const TYPES = ['row', 'inline', 'column', 'responsiveColumn'] as const;
Expand All @@ -56,3 +60,6 @@ export type EuiDescriptionListTextStyle = (typeof TEXT_STYLES)[number];

export const GUTTER_SIZES = ['s', 'm'] as const;
export type EuiDescriptionListGutterSizes = (typeof GUTTER_SIZES)[number];

export const COLUMN_GAP_SIZES = ['s', 'm'] as const;
export type EuiDescriptionListColumnGapSizes = (typeof GUTTER_SIZES)[number];
Loading