Skip to content
This repository was archived by the owner on Jan 14, 2025. It is now read-only.

Commit dd52744

Browse files
author
Matt Goo
authored
fix(list): maintain classes with state.listItemClassNames (#776)
BREAKING CHANGE: Removes props.tabbableOnListItemFocus from all the auxiliary components, as it seemed confusing to have this and tabIndex dictate what tabIndex would ultimately be.
1 parent 8064955 commit dd52744

14 files changed

+291
-199
lines changed

‎package-lock.json

Lines changed: 81 additions & 81 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/list/ListItem.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,9 @@ export interface ListItemProps<T> extends React.HTMLProps<T> {
3434
tag?: string;
3535
activated?: boolean;
3636
selected?: boolean;
37-
};
37+
onDestroy?: () => void;
38+
}
3839

39-
// TODO: convert to functional component
40-
// https://github.com/material-components/material-components-web-react/issues/729
4140
export default class ListItem<T extends HTMLElement = HTMLElement> extends React.Component<
4241
ListItemProps<T>,
4342
{}
@@ -53,14 +52,20 @@ export default class ListItem<T extends HTMLElement = HTMLElement> extends React
5352
onClick: () => {},
5453
onFocus: () => {},
5554
onBlur: () => {},
55+
onDestroy: () => {},
5656
tag: 'li',
5757
};
5858

59+
componentWillUnmount() {
60+
this.props.onDestroy!();
61+
}
62+
5963
get classes() {
60-
const {className, activated, selected} = this.props;
64+
const {className, activated, disabled, selected} = this.props;
6165
return classnames('mdc-list-item', className, {
6266
[MDCListFoundation.cssClasses.LIST_ITEM_ACTIVATED_CLASS]: activated,
6367
[MDCListFoundation.cssClasses.LIST_ITEM_SELECTED_CLASS]: selected,
68+
'mdc-list-item--disabled': disabled,
6469
});
6570
}
6671

@@ -84,9 +89,9 @@ export default class ListItem<T extends HTMLElement = HTMLElement> extends React
8489
role,
8590
checkboxList,
8691
radioList,
92+
onDestroy,
8793
/* eslint-enable no-unused-vars */
8894
tag: Tag,
89-
9095
...otherProps
9196
} = this.props;
9297
return (

‎packages/list/ListItemGraphic.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import * as React from 'react';
2424
import classnames from 'classnames';
2525

2626
export interface ListItemGraphicProps {
27-
tabbableOnListItemFocus?: boolean;
2827
className?: string;
2928
tabIndex?: number;
3029
graphic: React.ReactElement<any>;
@@ -34,13 +33,12 @@ export interface ListItemGraphicProps {
3433
const ListItemGraphic:React.FunctionComponent<ListItemGraphicProps> = ({
3534
tabIndex = -1, // eslint-disable-line no-unused-vars
3635
graphic,
37-
tabbableOnListItemFocus = false,
3836
className = '',
3937
...otherProps
4038
}) => {
4139
const graphicProps = {
4240
className: classnames('mdc-list-item__graphic', className),
43-
tabIndex: tabbableOnListItemFocus ? tabIndex : -1,
41+
tabIndex: tabIndex !== undefined ? tabIndex : -1,
4442
...otherProps,
4543
};
4644
return React.cloneElement(graphic, graphicProps);

‎packages/list/ListItemMeta.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,16 @@ import * as React from 'react';
2424
import classnames from 'classnames';
2525

2626
export interface ListItemMetaProps {
27-
tabbableOnListItemFocus?: boolean;
2827
className?: string;
2928
tabIndex?: number;
3029
meta: string | React.ReactElement<any>;
3130
childrenTabIndex?: number;
3231
};
3332

3433
const ListItemMeta:React.FunctionComponent<ListItemMetaProps> = ({
35-
tabIndex = -1, // eslint-disable-line no-unused-vars
34+
tabIndex, // eslint-disable-line no-unused-vars
3635
meta,
3736
className = '',
38-
tabbableOnListItemFocus = false,
3937
...otherProps
4038
}) => {
4139
let metaElement: React.ReactElement<any>;
@@ -46,7 +44,7 @@ const ListItemMeta:React.FunctionComponent<ListItemMetaProps> = ({
4644
}
4745
const metaProps = {
4846
className: classnames('mdc-list-item__meta', className, metaElement.props.className),
49-
tabIndex: tabbableOnListItemFocus ? tabIndex : -1,
47+
tabIndex: tabIndex !== undefined ? tabIndex : -1,
5048
...otherProps,
5149
};
5250
return React.cloneElement(metaElement, metaProps);

‎packages/list/ListItemText.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import * as React from 'react';
2424
import classnames from 'classnames';
2525

2626
export interface ListItemTextProps {
27-
tabbableOnListItemFocus?: boolean;
2827
tabIndex?: number;
2928
className?: string;
3029
primaryText?: React.ReactNode;
@@ -41,7 +40,6 @@ const ListItemText: React.FunctionComponent<ListItemTextProps> = ({
4140
/* eslint-disable react/prop-types */
4241
primaryText = '',
4342
secondaryText = '',
44-
tabbableOnListItemFocus = false,
4543
tabIndex = -1,
4644
className = '',
4745
/* eslint-enable react/prop-types */
@@ -53,7 +51,7 @@ const ListItemText: React.FunctionComponent<ListItemTextProps> = ({
5351
return (
5452
<span
5553
className={className}
56-
tabIndex={tabbableOnListItemFocus ? tabIndex : -1}
54+
tabIndex={tabIndex !== undefined ? tabIndex : -1}
5755
>
5856
{text}
5957
</span>
@@ -78,7 +76,7 @@ const ListItemText: React.FunctionComponent<ListItemTextProps> = ({
7876
return (
7977
<span
8078
className={classnames('mdc-list-item__text', className)}
81-
tabIndex={tabbableOnListItemFocus ? tabIndex : -1}
79+
tabIndex={tabIndex !== undefined ? tabIndex : -1}
8280
{...otherProps}
8381
>
8482
{renderText(primaryText, 'mdc-list-item__primary-text')}

‎packages/list/README.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,6 @@ class MyApp extends Component {
312312
| ----------------------- | ------- | ----------------------------------------------------------------------------------------------------------------- |
313313
| className | String | Classes to be applied to the list item text element |
314314
| tabIndex | Number | Tab index of the list item text |
315-
| tabbableOnListItemFocus | Boolean | Whether focusing list item will toggle tab index of the list item text. If false, the tab index will always be -1 |
316315
| primaryText | String | Primary text for the list item |
317316
| secondaryText | String | Secondary text for the list item |
318317

@@ -322,7 +321,6 @@ class MyApp extends Component {
322321
| ----------------------- | ------- | -------------------------------------------------------------------------------------------------------------------- |
323322
| className | String | Classes to be applied to the list item graphic element |
324323
| tabIndex | Number | Tab index of the list item graphic |
325-
| tabbableOnListItemFocus | Boolean | Whether focusing list item will toggle tab index of the list item graphic. If false, the tab index will always be -1 |
326324
| graphic | Element | The graphic element to be displayed in front of list item text |
327325

328326
### ListItemMeta
@@ -331,7 +329,6 @@ class MyApp extends Component {
331329
| ----------------------- | ----------------- | ----------------------------------------------------------------------------------------------------------------- |
332330
| className | String | Classes to be applied to the list item meta element |
333331
| tabIndex | Number | Tab index of the list item meta |
334-
| tabbableOnListItemFocus | Boolean | Whether focusing list item will toggle tab index of the list item meta. If false, the tab index will always be -1 |
335332
| meta | Element or String | The meta element or string to be displayed behind list item text |
336333

337334
### ListDivider

‎packages/list/index.tsx

Lines changed: 90 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ export interface ListProps<T extends HTMLElement> extends React.HTMLProps<HTMLEl
5151
children: ListItem<T> | ListItem<T>[] | React.ReactNode;
5252
};
5353

54+
55+
interface ListState {
56+
listItemClassNames: {[listItemIndex: number]: string[]},
57+
}
58+
5459
function isReactText(element: any): element is React.ReactText {
5560
return typeof element === 'string' || typeof element === 'number';
5661
}
@@ -63,13 +68,18 @@ function isSelectedIndexType(selectedIndex: unknown): selectedIndex is MDCListIn
6368
return typeof selectedIndex === 'number' && !isNaN(selectedIndex) || Array.isArray(selectedIndex);
6469
}
6570

66-
export default class List<T extends HTMLElement = HTMLElement> extends React.Component<ListProps<T>, {}> {
71+
export default class List<T extends HTMLElement = HTMLElement> extends React.Component<ListProps<T>, ListState> {
6772
listItemCount = 0;
6873
foundation!: MDCListFoundation;
69-
hasInitializedListItem = false;
74+
hasInitializedListItemTabIndex = false;
75+
hasInitializedList = false;
7076

7177
private listElement = React.createRef<HTMLElement>();
7278

79+
state: ListState = {
80+
listItemClassNames: {},
81+
};
82+
7383
static defaultProps: Partial<ListProps<HTMLElement>> = {
7484
'className': '',
7585
'checkboxList': false,
@@ -100,6 +110,11 @@ export default class List<T extends HTMLElement = HTMLElement> extends React.Com
100110
this.props[ARIA_ORIENTATION] === VERTICAL
101111
);
102112
this.initializeListType();
113+
114+
// tabIndex for the list items can only be initialized after
115+
// the above logic has executed. Once this is true, we need to call forceUpdate.
116+
this.hasInitializedList = true;
117+
this.forceUpdate();
103118
}
104119

105120
componentDidUpdate(prevProps: ListProps<T>) {
@@ -185,16 +200,30 @@ export default class List<T extends HTMLElement = HTMLElement> extends React.Com
185200
listItem.setAttribute(attr, value);
186201
}
187202
},
203+
/**
204+
* Pushes class name to state.listItemClassNames[listItemIndex] if it doesn't yet exist.
205+
*/
188206
addClassForElementIndex: (index, className) => {
189-
const listItem = this.listElements[index];
190-
if (listItem) {
191-
listItem.classList.add(className);
207+
const {listItemClassNames} = this.state;
208+
if (listItemClassNames[index] && listItemClassNames[index].indexOf(className) === -1) {
209+
listItemClassNames[index].push(className);
210+
} else {
211+
listItemClassNames[index] = [className];
192212
}
213+
this.setState({listItemClassNames});
193214
},
215+
/**
216+
* Finds the className within state.listItemClassNames[listItemIndex], and removes it
217+
* from the array.
218+
*/
194219
removeClassForElementIndex: (index, className) => {
195-
const listItem = this.listElements[index];
196-
if (listItem) {
197-
listItem.classList.remove(className);
220+
const {listItemClassNames} = this.state;
221+
if (listItemClassNames[index]) {
222+
const removalIndex = listItemClassNames[index].indexOf(className);
223+
if (removalIndex !== -1) {
224+
listItemClassNames[index].splice(removalIndex, 1);
225+
this.setState({listItemClassNames});
226+
}
198227
}
199228
},
200229
setTabIndexForListItemChildren: (listItemIndex, tabIndexValue) => {
@@ -251,6 +280,44 @@ export default class List<T extends HTMLElement = HTMLElement> extends React.Com
251280
return null;
252281
}
253282

283+
/**
284+
* Initializes the tabIndex prop for the listItems. tabIndex is determined by:
285+
* 1. if selectedIndex is an array, and the index === selectedIndex[0]
286+
* 2. if selectedIndex is a number, and the the index === selectedIndex
287+
* 3. if there is no selectedIndex
288+
*/
289+
private getListItemInitialTabIndex = (index: number) => {
290+
const {selectedIndex} = this.props;
291+
let tabIndex = {};
292+
if (this.hasInitializedList && !this.hasInitializedListItemTabIndex) {
293+
const isSelectedIndexArray
294+
= Array.isArray(selectedIndex) && selectedIndex.length > 0 && index === selectedIndex[0];
295+
const isSelected = selectedIndex === index;
296+
if (isSelectedIndexArray || isSelected || selectedIndex === -1) {
297+
tabIndex = {tabIndex: 0};
298+
this.hasInitializedListItemTabIndex = true;
299+
}
300+
}
301+
302+
return tabIndex;
303+
}
304+
305+
/**
306+
* Method checks if the list item at `index` contains classes. If true,
307+
* method merges state.listItemClassNames[index] with listItem.props.className.
308+
* The return value is used as the listItem's className.
309+
*/
310+
private getListItemClassNames = (index: number, listItem: React.ReactElement<ListItemProps<T>>) => {
311+
let {className = ''} = listItem.props;
312+
const {listItemClassNames} = this.state;
313+
if (listItemClassNames[index]) {
314+
listItemClassNames[index];
315+
className = classnames(className, listItemClassNames[index]);
316+
}
317+
318+
return className;
319+
}
320+
254321
handleKeyDown = (e: React.KeyboardEvent<any>, index: number) => {
255322
e.persist(); // Persist the synthetic event to access its `key`
256323
this.foundation.handleKeydown(
@@ -317,46 +384,33 @@ export default class List<T extends HTMLElement = HTMLElement> extends React.Com
317384

318385
renderChild = (child: React.ReactElement<ListItemProps<T>> | React.ReactChild) => {
319386
if (!isReactText(child) && isListItem(child)) {
320-
return this.renderListItem(child, this.listItemCount++);
387+
return this.renderListItem(child, child.props.disabled ? -1 : this.listItemCount++);
321388
}
322389
return child;
323390
};
324391

325392
renderListItem = (listItem: React.ReactElement<ListItemProps<T>>, index: number) => {
326-
const {checkboxList, radioList, selectedIndex} = this.props;
327-
let tabIndex = {};
393+
const {checkboxList, radioList} = this.props;
394+
const tabIndex = this.getListItemInitialTabIndex(index);
395+
const className = this.getListItemClassNames(index, listItem);
396+
328397
const {
329398
onKeyDown,
330399
onClick,
331400
onFocus,
332401
onBlur,
402+
/* eslint-disable no-unused-vars */
403+
className: _classNames,
404+
/* eslint-enable no-unused-vars */
333405
...otherProps
334406
} = listItem.props;
335407

336-
if (!this.hasInitializedListItem) {
337-
const isSelectedIndexArray = Array.isArray(selectedIndex) && selectedIndex.length > 0;
338-
// if selectedIndex is populated then check if its a checkbox/radioList
339-
if (selectedIndex && (isSelectedIndexArray || selectedIndex > -1)) {
340-
const isCheckboxListSelected
341-
= checkboxList && Array.isArray(selectedIndex) && selectedIndex.indexOf(index) > -1;
342-
const isNonCheckboxListSelected = selectedIndex === index;
343-
if (isCheckboxListSelected || isNonCheckboxListSelected) {
344-
tabIndex = {tabIndex: 0};
345-
this.hasInitializedListItem = true;
346-
}
347-
// set tabIndex=0 to first listItem if selectedIndex is not populated
348-
} else {
349-
tabIndex = {tabIndex: 0};
350-
this.hasInitializedListItem = true;
351-
}
352-
}
353-
354-
355408
const props = {
356-
// otherProps must come first
409+
// otherProps must be first
357410
...otherProps,
358411
checkboxList,
359412
radioList,
413+
className,
360414
onKeyDown: (e: React.KeyboardEvent<T>) => {
361415
onKeyDown!(e);
362416
this.handleKeyDown(e, index);
@@ -373,6 +427,11 @@ export default class List<T extends HTMLElement = HTMLElement> extends React.Com
373427
onBlur!(e);
374428
this.handleBlur(e, index);
375429
},
430+
onDestroy: () => {
431+
const {listItemClassNames} = this.state;
432+
delete listItemClassNames[index];
433+
this.setState({listItemClassNames});
434+
},
376435
...tabIndex,
377436
};
378437
return React.cloneElement(listItem, props);

‎test/screenshot/golden.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"layout-grid": "fe40f7a34853bc2a1d9a836e812599d4d47b5b26b839a8eaed7f98ea91946790",
1010
"line-ripple": "56b136db2dc7e09260849447e6bde9b55a837af332a05d9f52506ab1c95e2e57",
1111
"linear-progress": "f7a23058842b37875a02b3562e0b9c82f7fb24f9b88e9727d10b921b001d115e",
12-
"list": "4dcdfeb03d781eca5367f6cbf4c7ebd13d1fe8932ca5758520b994d541f49095",
12+
"list": "e633e9536eeadfe7bbb46cdf3d42c5ebf40a8974420f6183057b6f7ddcbeca21",
1313
"material-icon": "442b39fb22d2c7a74efb23ca098429b471501ce21df8662327bbf9871fe0bcb0",
1414
"menu-surface": "f5face1a24fe166e86e8a3dc35ea85b2d4431469a3d06bf6fc1a30fbdc175aff",
1515
"notched-outline": "7770dd381c27608a1f43b6f83da92507fe53963f5e4409bd73184b86275538fe",

0 commit comments

Comments
 (0)