Skip to content

Commit c4fe12a

Browse files
rh-gvincentrh-gvincent
committed
refactor: Use the same mechanism for autogenerated component id
To make the code more consistent, I added a method getUniqueId that is used by all components that need a unique id. I put this id recovery in the defaultProps object. Add also some tests for util file.
1 parent 4926d70 commit c4fe12a

File tree

7 files changed

+61
-59
lines changed

7 files changed

+61
-59
lines changed

‎packages/patternfly-4/react-core/src/components/Dropdown/Dropdown.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { css } from '@patternfly/react-styles';
44
import PropTypes from 'prop-types';
55
import FocusTrap from 'focus-trap-react';
66
import DropdownMenu from './DropdownMenu';
7+
import { getUniqueId } from '../../internal/util';
78

89
export const DropdownPosition = {
910
right: 'right',
@@ -37,11 +38,7 @@ const propTypes = {
3738
};
3839

3940
const defaultProps = {
40-
id:
41-
new Date().getTime() +
42-
Math.random()
43-
.toString(36)
44-
.slice(2),
41+
id: getUniqueId(),
4542
toggle: null,
4643
children: null,
4744
className: '',

‎packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import PropTypes from 'prop-types';
66
import NavToggle from './NavToggle';
77
import { AngleRightIcon } from '@patternfly/react-icons';
88
import { NavContext } from './Nav';
9+
import { getUniqueId } from '../../internal/util';
910

1011
const propTypes = {
1112
/** Title shown for the expandable list */
@@ -27,29 +28,28 @@ const propTypes = {
2728
};
2829

2930
const defaultProps = {
31+
id: getUniqueId(),
3032
srText: '',
3133
isExpanded: false,
3234
children: null,
3335
className: '',
3436
groupId: null,
35-
isActive: false,
36-
id: ''
37+
isActive: false
3738
};
3839

3940
class NavExpandable extends React.Component {
40-
constructor(props) {
41-
super(props);
42-
43-
this.uniqueId =
44-
props.id ||
45-
new Date().getTime() +
46-
Math.random()
47-
.toString(36)
48-
.slice(2);
49-
}
50-
5141
render() {
52-
const { title, srText, isExpanded: defaultExpanded, children, className, groupId, isActive, ...props } = this.props;
42+
const {
43+
id,
44+
title,
45+
srText,
46+
isExpanded: defaultExpanded,
47+
children,
48+
className,
49+
groupId,
50+
isActive,
51+
...props
52+
} = this.props;
5353

5454
return (
5555
<NavContext.Consumer>
@@ -68,7 +68,7 @@ class NavExpandable extends React.Component {
6868
>
6969
<a
7070
className={css(styles.navLink)}
71-
id={srText ? null : this.uniqueId}
71+
id={srText ? null : id}
7272
href="#"
7373
onClick={e => e.preventDefault()}
7474
onMouseDown={e => e.preventDefault()}
@@ -81,11 +81,11 @@ class NavExpandable extends React.Component {
8181
</a>
8282
<section
8383
className={css(styles.navSubnav)}
84-
aria-labelledby={this.uniqueId}
84+
aria-labelledby={id}
8585
hidden={isExpanded ? null : true}
8686
>
8787
{srText && (
88-
<h2 className={css(a11yStyles.srOnly)} id={this.uniqueId}>
88+
<h2 className={css(a11yStyles.srOnly)} id={id}>
8989
{srText}
9090
</h2>
9191
)}

‎packages/patternfly-4/react-core/src/components/Nav/NavGroup.js‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import React from 'react';
22
import styles from '@patternfly/patternfly-next/components/Nav/nav.css';
33
import { css } from '@patternfly/react-styles';
44
import PropTypes from 'prop-types';
5+
import { getUniqueId } from '../../internal/util';
56

67
const propTypes = {
78
/** Title shown for the group */
@@ -15,29 +16,18 @@ const propTypes = {
1516
};
1617

1718
const defaultProps = {
19+
id: getUniqueId(),
1820
children: null,
19-
className: '',
20-
id: ''
21+
className: ''
2122
};
2223

2324
class NavGroup extends React.Component {
24-
constructor(props) {
25-
super(props);
26-
27-
this.uniqueId =
28-
props.id ||
29-
new Date().getTime() +
30-
Math.random()
31-
.toString(36)
32-
.slice(2);
33-
}
34-
3525
render() {
36-
const { title, children, className, ...props } = this.props;
26+
const { id, title, children, className, ...props } = this.props;
3727

3828
return (
39-
<section className={css(styles.navSection, className)} aria-labelledby={this.uniqueId} {...props}>
40-
<h2 className={css(styles.navSectionTitle)} id={this.uniqueId}>
29+
<section className={css(styles.navSection, className)} aria-labelledby={id} {...props}>
30+
<h2 className={css(styles.navSectionTitle)} id={id}>
4131
{title}
4232
</h2>
4333
<ul className={css(styles.navSimpleList)}>{children}</ul>

‎packages/patternfly-4/react-core/src/components/Nav/__snapshots__/Nav.test.js.snap‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,6 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
378378
>
379379
<li
380380
className="pf-c-nav__item expandable-group"
381-
id="grp-1"
382381
onClick={[Function]}
383382
>
384383
<a
@@ -613,7 +612,6 @@ exports[`Expandable Nav List 1`] = `
613612
>
614613
<li
615614
className="pf-c-nav__item"
616-
id="grp-1"
617615
onClick={[Function]}
618616
>
619617
<a
@@ -856,7 +854,6 @@ exports[`Expandable Nav List with aria label 1`] = `
856854
>
857855
<li
858856
className="pf-c-nav__item"
859-
id="grp-1"
860857
onClick={[Function]}
861858
>
862859
<a
@@ -1251,7 +1248,6 @@ exports[`Nav Grouped List 1`] = `
12511248
<section
12521249
aria-labelledby="grp-1"
12531250
className="pf-c-nav__section"
1254-
id="grp-1"
12551251
>
12561252
<h2
12571253
className="pf-c-nav__section-title"
@@ -1366,7 +1362,6 @@ exports[`Nav Grouped List 1`] = `
13661362
<section
13671363
aria-labelledby="grp-2"
13681364
className="pf-c-nav__section"
1369-
id="grp-2"
13701365
>
13711366
<h2
13721367
className="pf-c-nav__section-title"

‎packages/patternfly-4/react-core/src/components/Progress/Progress.js‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import styles from '@patternfly/patternfly-next/components/Progress/progress.css
33
import { css, getModifier } from '@patternfly/react-styles';
44
import PropTypes from 'prop-types';
55
import ProgressContainer, { ProgressMeasureLocation, ProgressVariant } from './ProgressContainer';
6+
import { getUniqueId } from '../../internal/util';
67

78
export const ProgressSize = {
89
sm: 'sm',
@@ -36,10 +37,10 @@ const propTypes = {
3637
};
3738

3839
const defaultProps = {
40+
id: getUniqueId(),
3941
className: '',
4042
measureLocation: ProgressMeasureLocation.top,
4143
variant: ProgressVariant.info,
42-
id: '',
4344
title: '',
4445
min: 0,
4546
max: 100,
@@ -49,20 +50,21 @@ const defaultProps = {
4950
};
5051

5152
class Progress extends Component {
52-
constructor(props) {
53-
super(props);
54-
this.state = {
55-
uniqueId:
56-
props.id ||
57-
`progress-${new Date().getTime()}${Math.random()
58-
.toString(36)
59-
.slice(2)}`
60-
};
61-
}
62-
6353
render() {
64-
const { className, size, id, value, title, label, variant, measureLocation, min, max, valueText, ...props } = this.props;
65-
const { uniqueId } = this.state;
54+
const {
55+
id,
56+
className,
57+
size,
58+
value,
59+
title,
60+
label,
61+
variant,
62+
measureLocation,
63+
min,
64+
max,
65+
valueText,
66+
...props
67+
} = this.props;
6668
const additionalProps = {
6769
...props,
6870
...(valueText ? { 'aria-valuetext': valueText } : { 'aria-describedby': `${id}-description` })
@@ -80,14 +82,14 @@ class Progress extends Component {
8082
getModifier(styles, measureLocation === ProgressMeasureLocation.inside ? ProgressSize.lg : size, ''),
8183
className
8284
)}
83-
id={uniqueId}
85+
id={id}
8486
role="progressbar"
8587
aria-valuemin={min}
8688
aria-valuenow={limitedValue}
8789
aria-valuemax={max}
8890
>
8991
<ProgressContainer
90-
parentId={uniqueId}
92+
parentId={id}
9193
value={limitedValue}
9294
title={title}
9395
label={label}

‎packages/patternfly-4/react-core/src/internal/util.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
11
export function capitalize(input) {
22
return input[0].toUpperCase() + input.substring(1);
33
}
4+
5+
export function getUniqueId() {
6+
return (
7+
new Date().getTime() +
8+
Math.random()
9+
.toString(36)
10+
.slice(2)
11+
);
12+
}

‎packages/patternfly-4/react-core/src/internal/util.test.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { capitalize, getUniqueId } from './util';
2+
3+
test('capitalize', () => {
4+
expect(capitalize('foo')).toBe('Foo');
5+
});
6+
7+
test('getUniqueId', () => {
8+
expect(getUniqueId()).not.toBe(getUniqueId());
9+
});

0 commit comments

Comments
 (0)