Skip to content

Commit 458aafd

Browse files
cee-chentkajtoch
authored andcommitted
[EuiListGroup] Memoize styles + clean up code
- `list_group_item` has too much conditional JSX to deal with right now, should likely be split up into subcomponents in the future + remove unnecessary style functions, just use a static obj instead + re-run VRT screenshots
1 parent ae07f8a commit 458aafd

File tree

6 files changed

+113
-132
lines changed

6 files changed

+113
-132
lines changed

packages/eui/src/components/list_group/list_group.tsx

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,19 @@
66
* Side Public License, v 1.
77
*/
88

9-
import React, { FunctionComponent, HTMLAttributes, CSSProperties } from 'react';
9+
import React, {
10+
FunctionComponent,
11+
HTMLAttributes,
12+
CSSProperties,
13+
useMemo,
14+
} from 'react';
1015
import classNames from 'classnames';
1116

12-
import { EuiListGroupItem, EuiListGroupItemProps } from './list_group_item';
13-
import { CommonProps } from '../common';
14-
import { useEuiTheme, cloneElementWithCss } from '../../services';
17+
import { useEuiMemoizedStyles, cloneElementWithCss } from '../../services';
1518
import { logicalStyle } from '../../global_styling';
19+
import { CommonProps } from '../common';
1620

21+
import { EuiListGroupItem, EuiListGroupItemProps } from './list_group_item';
1722
import { euiListGroupStyles } from './list_group.styles';
1823

1924
export const GUTTER_SIZES = ['none', 's', 'm'] as const;
@@ -89,17 +94,8 @@ export const EuiListGroup: FunctionComponent<EuiListGroupProps> = ({
8994
ariaLabelledby,
9095
...rest
9196
}) => {
92-
let newStyle = style;
93-
94-
if (maxWidth && maxWidth !== true) {
95-
newStyle = { ...newStyle, ...logicalStyle('max-width', maxWidth) };
96-
}
97-
9897
const classes = classNames('euiListGroup', className);
99-
100-
const euiTheme = useEuiTheme();
101-
const styles = euiListGroupStyles(euiTheme);
102-
98+
const styles = useEuiMemoizedStyles(euiListGroupStyles);
10399
const cssStyles = [
104100
styles.euiListGroup,
105101
styles[gutterSize],
@@ -108,11 +104,15 @@ export const EuiListGroup: FunctionComponent<EuiListGroupProps> = ({
108104
maxWidth === true && styles.maxWidthDefault,
109105
];
110106

111-
let childrenOrListItems = null;
107+
const maxWidthStyle = useMemo(() => {
108+
if (maxWidth && maxWidth !== true) {
109+
return logicalStyle('max-width', maxWidth);
110+
}
111+
}, [maxWidth]);
112112

113-
if (listItems) {
114-
childrenOrListItems = listItems.map((item, index) => {
115-
return [
113+
const renderedListItems = useMemo(() => {
114+
if (listItems && listItems.length) {
115+
return listItems.map((item, index) => (
116116
<EuiListGroupItem
117117
key={`title-${index}`}
118118
showToolTip={showToolTips}
@@ -122,35 +122,36 @@ export const EuiListGroup: FunctionComponent<EuiListGroupProps> = ({
122122
color={color}
123123
size={size}
124124
{...item}
125-
/>,
126-
];
127-
});
128-
} else {
129-
const showToolTipObj = showToolTips && { showToolTip: true };
130-
131-
childrenOrListItems = React.Children.map(children, (child) => {
125+
/>
126+
));
127+
}
128+
}, [listItems, color, size, wrapText, showToolTips]);
129+
130+
const listItemsOrChildren =
131+
renderedListItems ||
132+
// Note that there's no point in memoizing `children`, as JSX changes every rerender
133+
React.Children.map(children, (child) => {
132134
if (React.isValidElement(child)) {
133135
return cloneElementWithCss(child, {
134136
// we're passing the parent `color` and `size` down to the children
135137
// so that they can inherit it if they don't have one
136138
color: color,
137139
size: size,
138-
...showToolTipObj,
140+
...(showToolTips && { showToolTip: true }),
139141
...child.props,
140142
});
141143
}
142144
});
143-
}
144145

145146
return (
146147
<ul
147148
className={classes}
148149
css={cssStyles}
149-
style={newStyle}
150+
style={{ ...style, ...maxWidthStyle }}
150151
aria-labelledby={ariaLabelledby}
151152
{...rest}
152153
>
153-
{childrenOrListItems}
154+
{listItemsOrChildren}
154155
</ul>
155156
);
156157
};

packages/eui/src/components/list_group/list_group_item.styles.ts

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,15 @@ export const euiListGroupItemInnerStyles = (euiThemeContext: UseEuiTheme) => {
170170
};
171171
};
172172

173-
export const euiListGroupItemLabelStyles = () => {
174-
return {
175-
// Base
176-
euiListGroupItem__label: css``,
177-
truncate: css`
178-
${euiTextTruncate()}
179-
`,
180-
wrapText: css`
181-
${euiTextBreakWord()}
182-
`,
183-
};
173+
export const euiListGroupItemLabelStyles = {
174+
// Base
175+
euiListGroupItem__label: css``,
176+
truncate: css`
177+
${euiTextTruncate()}
178+
`,
179+
wrapText: css`
180+
${euiTextBreakWord()}
181+
`,
184182
};
185183

186184
export const euiListGroupItemIconStyles = ({ euiTheme }: UseEuiTheme) => {
@@ -194,12 +192,10 @@ export const euiListGroupItemIconStyles = ({ euiTheme }: UseEuiTheme) => {
194192
};
195193
};
196194

197-
export const euiListGroupItemTooltipStyles = () => {
198-
return {
199-
// Base
200-
euiListGroupItem__tooltip: css`
201-
display: inline-flex; /* Allows the wrapped button/text to grow */
202-
${logicalCSS('width', '100%')}
203-
`,
204-
};
195+
export const euiListGroupItemTooltipStyles = {
196+
// Base
197+
euiListGroupItem__tooltip: css`
198+
display: inline-flex; /* Allows the wrapped button/text to grow */
199+
${logicalCSS('width', '100%')}
200+
`,
205201
};

packages/eui/src/components/list_group/list_group_item.tsx

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
import React, {
10-
Fragment,
1110
HTMLAttributes,
1211
AnchorHTMLAttributes,
1312
ButtonHTMLAttributes,
@@ -18,23 +17,22 @@ import React, {
1817
} from 'react';
1918
import classNames from 'classnames';
2019

21-
import { EuiIcon, IconType, EuiIconProps } from '../icon';
22-
import { EuiToolTip, EuiToolTipProps } from '../tool_tip';
23-
import { useInnerText } from '../inner_text';
24-
import { ExclusiveUnion, CommonProps } from '../common';
25-
import {
26-
EuiListGroupItemExtraAction,
27-
EuiListGroupItemExtraActionProps,
28-
} from './list_group_item_extra_action';
29-
3020
import {
3121
getSecureRelForTarget,
32-
useEuiTheme,
22+
useEuiMemoizedStyles,
3323
cloneElementWithCss,
3424
} from '../../services';
3525
import { validateHref } from '../../services/security/href_validator';
26+
import { ExclusiveUnion, CommonProps } from '../common';
27+
import { useInnerText } from '../inner_text';
28+
import { EuiIcon, IconType, EuiIconProps } from '../icon';
29+
import { EuiToolTip, EuiToolTipProps } from '../tool_tip';
3630
import { EuiExternalLinkIcon } from '../link/external_link_icon';
3731

32+
import {
33+
EuiListGroupItemExtraAction,
34+
EuiListGroupItemExtraActionProps,
35+
} from './list_group_item_extra_action';
3836
import {
3937
euiListGroupItemStyles,
4038
euiListGroupItemIconStyles,
@@ -185,9 +183,7 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
185183
const isHrefValid = !href || validateHref(href);
186184
const isDisabled = _isDisabled || !isHrefValid;
187185

188-
const euiTheme = useEuiTheme();
189-
190-
const iconStyles = euiListGroupItemIconStyles(euiTheme);
186+
const iconStyles = useEuiMemoizedStyles(euiListGroupItemIconStyles);
191187
const cssIconStyles = [iconStyles.euiListGroupItem__icon, iconProps?.css];
192188

193189
let iconNode;
@@ -243,7 +239,7 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
243239
);
244240
}
245241

246-
const labelStyles = euiListGroupItemLabelStyles();
242+
const labelStyles = euiListGroupItemLabelStyles;
247243
const cssLabelStyles = [
248244
labelStyles.euiListGroupItem__label,
249245
wrapText ? labelStyles.wrapText : labelStyles.truncate,
@@ -271,7 +267,7 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
271267
// Handle the variety of interaction behavior
272268
let itemContent;
273269

274-
const innerStyles = euiListGroupItemInnerStyles(euiTheme);
270+
const innerStyles = useEuiMemoizedStyles(euiListGroupItemInnerStyles);
275271
const cssInnerStyles = [
276272
innerStyles.euiListGroupItem__inner,
277273
innerStyles[size],
@@ -321,7 +317,7 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
321317
);
322318
}
323319

324-
const styles = euiListGroupItemStyles(euiTheme);
320+
const styles = useEuiMemoizedStyles(euiListGroupItemStyles);
325321
const cssStyles = [
326322
styles.euiListGroupItem,
327323
!isDisabled && isActive && styles.colors.isActive[color],
@@ -333,7 +329,7 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
333329
const classes = classNames('euiListGroupItem', className);
334330

335331
if (showToolTip) {
336-
const tooltipStyles = euiListGroupItemTooltipStyles();
332+
const tooltipStyles = euiListGroupItemTooltipStyles;
337333
const cssTooltipStyles = [
338334
tooltipStyles.euiListGroupItem__tooltip,
339335
toolTipProps?.anchorProps?.css,
@@ -375,5 +371,5 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
375371
);
376372
}
377373

378-
return <Fragment>{itemContent}</Fragment>;
374+
return <>{itemContent}</>;
379375
};

packages/eui/src/components/list_group/list_group_item_extra_action.test.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@ describe('EuiListGroupItem', () => {
1919
iconType: 'star',
2020
};
2121

22-
shouldRenderCustomStyles(<EuiListGroupItemExtraAction iconType="star" />);
22+
shouldRenderCustomStyles(<EuiListGroupItemExtraAction {...props} />);
2323

2424
test('is rendered', () => {
25-
const { container } = render(
26-
<EuiListGroupItemExtraAction {...requiredProps} iconType="star" />
27-
);
25+
const { container } = render(<EuiListGroupItemExtraAction {...props} />);
2826

2927
expect(container.firstChild).toMatchSnapshot();
3028
});

packages/eui/src/components/list_group/list_group_item_extra_action.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
import React, { FunctionComponent } from 'react';
1010
import classNames from 'classnames';
1111

12+
import { useEuiMemoizedStyles } from '../../services';
1213
import { EuiButtonIcon, EuiButtonIconPropsForButton } from '../button';
1314

14-
import { useEuiTheme } from '../../services';
1515
import { euiListGroupItemExtraActionStyles } from './list_group_item_extra_action.styles';
1616

1717
export type EuiListGroupItemExtraActionProps = {
@@ -36,9 +36,9 @@ export const EuiListGroupItemExtraAction: FunctionComponent<
3636
className
3737
);
3838

39-
const euiTheme = useEuiTheme();
40-
41-
const extraActionStyles = euiListGroupItemExtraActionStyles(euiTheme);
39+
const extraActionStyles = useEuiMemoizedStyles(
40+
euiListGroupItemExtraActionStyles
41+
);
4242
const cssExtraActionStyles = [
4343
extraActionStyles.euiListGroupItemExtraAction,
4444
alwaysShow && extraActionStyles.alwaysShow,

0 commit comments

Comments
 (0)