Skip to content

Commit ec9e566

Browse files
rebeccaalpertrebeccaalpert
committed
Address PR feedback
1 parent e51d209 commit ec9e566

File tree

8 files changed

+15
-20
lines changed

8 files changed

+15
-20
lines changed

‎packages/react-core/src/components/JumpLinks/JumpLinks.tsx

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ export interface JumpLinksProps extends Omit<React.HTMLProps<HTMLElement>, 'labe
4747
className?: string;
4848
/** Whether the current entry in the navigation history should be replaced when a JumpLinksItem is clicked. By default a new entry will be pushed to the navigation history. */
4949
shouldReplaceNavHistory?: boolean;
50+
/** Custom ID applied to label if alwaysShowLabel is applied, or expandable toggle. This is used for internal logic related to aria-label and aria-labelledby */
51+
labelId?: string;
5052
}
5153

5254
// Recursively find JumpLinkItems and return an array of all their scrollNodes
@@ -94,6 +96,7 @@ export const JumpLinks: React.FunctionComponent<JumpLinksProps> = ({
9496
toggleAriaLabel = 'Toggle jump links',
9597
className,
9698
shouldReplaceNavHistory = false,
99+
labelId,
97100
...props
98101
}: JumpLinksProps) => {
99102
const hasScrollSpy = Boolean(scrollableRef || scrollableSelector);
@@ -108,13 +111,11 @@ export const JumpLinks: React.FunctionComponent<JumpLinksProps> = ({
108111

109112
if (!label && !ariaLabel) {
110113
// eslint-disable-next-line no-console
111-
console.warn('For accessibility reasons, an aria-label should be specified on jump links if no label is provided');
114+
console.warn('JumpLinks: for accessibility reasons, an aria-label should be specified if no label is provided');
112115
}
113-
if (!label && !toggleAriaLabel) {
116+
if (!label && !toggleAriaLabel && expandable) {
114117
// eslint-disable-next-line no-console
115-
console.warn(
116-
'For accessibility reasons, a toggleAriaLabel should be specified on jump links if no label is provided'
117-
);
118+
console.warn('JumpLinks: for accessibility reasons, a toggleAriaLabel should be specified if no label is provided');
118119
}
119120

120121
const getScrollableElement = () => {
@@ -243,7 +244,7 @@ export const JumpLinks: React.FunctionComponent<JumpLinksProps> = ({
243244
return child;
244245
});
245246

246-
const id = getUniqueId();
247+
const id = labelId ?? getUniqueId();
247248
const hasAriaLabelledBy = expandable || (label && alwaysShowLabel);
248249
const computedAriaLabel = hasAriaLabelledBy ? null : ariaLabel;
249250
const computedAriaLabelledBy = hasAriaLabelledBy ? id : null;
@@ -283,7 +284,7 @@ export const JumpLinks: React.FunctionComponent<JumpLinksProps> = ({
283284
</Button>
284285
</div>
285286
)}
286-
{label && alwaysShowLabel && (
287+
{label && alwaysShowLabel && !expandable && (
287288
<div className={css(styles.jumpLinksLabel)} id={id}>
288289
{label}
289290
</div>

‎packages/react-core/src/components/JumpLinks/__tests__/__snapshots__/JumpLinks.test.tsx.snap

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ exports[`expandable vertical with subsection 1`] = `
5252
</span>
5353
</button>
5454
</div>
55-
<div
56-
class="pf-v6-c-jump-links__label"
57-
id="unique_id_mock"
58-
>
59-
Jump to section
60-
</div>
6155
</div>
6256
<ul
6357
aria-labelledby="unique_id_mock"

‎packages/react-core/src/components/JumpLinks/examples/JumpLinksBasic.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { JumpLinks, JumpLinksItem } from '@patternfly/react-core';
22

33
export const JumpLinksBasic: React.FunctionComponent = () => (
4-
<JumpLinks aria-label="Basic example">
4+
<JumpLinks aria-label="Jump to basic example sections">
55
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
66
<JumpLinksItem href="#" isActive>
77
Active section

‎packages/react-core/src/components/JumpLinks/examples/JumpLinksExpandableVerticalWithSubsection.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ export const JumpLinksExpandableVerticalWithSubsection: React.FunctionComponent
55
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
66
<JumpLinksItem href="#">
77
Section with active subsection
8-
<JumpLinksList>
8+
<JumpLinksList aria-label="Expandable vertical section subsection">
99
<JumpLinksItem href="#" isActive>
1010
Active subsection
1111
</JumpLinksItem>

‎packages/react-core/src/components/JumpLinks/examples/JumpLinksVertical.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { JumpLinks, JumpLinksItem } from '@patternfly/react-core';
22

33
export const JumpLinksVertical: React.FunctionComponent = () => (
4-
<JumpLinks isVertical aria-label="Vertical example">
4+
<JumpLinks isVertical aria-label="Jump to vertical example sections">
55
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
66
<JumpLinksItem href="#" isActive>
77
Active section

‎packages/react-core/src/components/JumpLinks/examples/JumpLinksVerticalWithLabel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { JumpLinks, JumpLinksItem } from '@patternfly/react-core';
22

33
export const JumpLinksVerticalWithLabel: React.FunctionComponent = () => (
4-
<JumpLinks isVertical label="Jump to vertical section">
4+
<JumpLinks isVertical label="Jump to vertical example sections with labels">
55
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
66
<JumpLinksItem href="#" isActive>
77
Active section

‎packages/react-core/src/components/JumpLinks/examples/JumpLinksWithCenteredList.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { JumpLinks, JumpLinksItem } from '@patternfly/react-core';
22

33
export const JumpLinksWithCenteredList: React.FunctionComponent = () => (
4-
<JumpLinks isCentered aria-label="Centered example">
4+
<JumpLinks isCentered aria-label="Jump to centered example sections">
55
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
66
<JumpLinksItem href="#" isActive>
77
Active section

‎packages/react-core/src/components/JumpLinks/examples/JumpLinksWithLabel.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ import { JumpLinks, JumpLinksItem } from '@patternfly/react-core';
22

33
export const JumpLinksWithLabel: React.FunctionComponent = () => (
44
<>
5-
<JumpLinks label="Jump to section">
5+
<JumpLinks label="Jump to first section in jump links with label example">
66
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
77
<JumpLinksItem href="#" isActive>
88
Active section
99
</JumpLinksItem>
1010
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
1111
</JumpLinks>
12-
<JumpLinks isCentered label="Jump to section">
12+
<JumpLinks isCentered label="Jump to second section in jump links with label example">
1313
<JumpLinksItem href="#">Inactive section</JumpLinksItem>
1414
<JumpLinksItem href="#" isActive>
1515
Active section

0 commit comments

Comments
 (0)