Skip to content

Commit 454aa8c

Browse files
kmcfaulkmcfaul
committed
pr updates from rabbit
1 parent 42b7e54 commit 454aa8c

File tree

5 files changed

+69
-76
lines changed

5 files changed

+69
-76
lines changed

‎packages/react-core/src/components/Compass/examples/Compass.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ When `footer` is used, its content will fill the width of the screen. By default
6060

6161
### With docked nav
6262

63-
As an alternative navigation, a `CompassDock` component may be passed to `Compass` via the `dock` prop. This component will allocate a thin sidebar intended for icons to the start of the screen. The `CompassDock` component has three sub-areas, from top to bottom: logo, main, and tools. Typically a `Brand` or other logo should be passed to the `logo` prop. The `main` and `tools` are flexible and can be passed `Nav`, `ActionList`, or `Toolbar` vertical variants depending on the use case. Account or profile avatars and links would typically be passed as part of the `tools` section at the bottom of the page.
64-
6563
```ts file="CompassDockLayout.tsx"
6664

6765
```

‎packages/react-core/src/components/Compass/examples/CompassDockDemo.tsx‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useRef, useState, useEffect } from 'react';
1+
import { useRef, useState } from 'react';
22
import {
33
Compass,
44
CompassContent,
@@ -46,9 +46,6 @@ interface NavOnSelectProps {
4646
export const CompassDockDemo: React.FunctionComponent = () => {
4747
const [activeItem, setActiveItem] = useState<number>(0);
4848
const [isDropdownOpen, setIsDropdownOpen] = useState(false);
49-
const [isOpen, setIsOpen] = useState<boolean>(false);
50-
const menuRef = useRef<HTMLDivElement>(null);
51-
const toggleRef = useRef<HTMLButtonElement>(null);
5249

5350
const onNavSelect = (_event: React.FormEvent<HTMLInputElement>, selectedItem: NavOnSelectProps) => {
5451
typeof selectedItem.itemId === 'number' && setActiveItem(selectedItem.itemId);
@@ -62,34 +59,6 @@ export const CompassDockDemo: React.FunctionComponent = () => {
6259
setIsDropdownOpen(!isDropdownOpen);
6360
};
6461

65-
const handleMenuKeys = (event: KeyboardEvent) => {
66-
if (!isOpen) {
67-
return;
68-
}
69-
if (menuRef.current?.contains(event.target as Node) || toggleRef.current?.contains(event.target as Node)) {
70-
if (event.key === 'Escape') {
71-
setIsOpen(!isOpen);
72-
toggleRef.current?.focus();
73-
}
74-
}
75-
};
76-
77-
const handleClickOutside = (event: MouseEvent) => {
78-
if (isOpen && !menuRef.current?.contains(event.target as Node)) {
79-
setIsOpen(false);
80-
}
81-
};
82-
83-
useEffect(() => {
84-
window.addEventListener('keydown', handleMenuKeys);
85-
window.addEventListener('click', handleClickOutside);
86-
87-
return () => {
88-
window.removeEventListener('keydown', handleMenuKeys);
89-
window.removeEventListener('click', handleClickOutside);
90-
};
91-
}, [isOpen, menuRef]);
92-
9362
const userDropdownItems = [
9463
<>
9564
<DropdownItem key="group 2 profile">My profile</DropdownItem>
@@ -127,6 +96,7 @@ export const CompassDockDemo: React.FunctionComponent = () => {
12796
isActive={activeItem === 0}
12897
icon={<CubeIcon />}
12998
ref={navItem1Ref}
99+
aria-label="Link 1"
130100
/>
131101
<NavItem
132102
key="nav-icon-link2"
@@ -137,26 +107,29 @@ export const CompassDockDemo: React.FunctionComponent = () => {
137107
isActive={activeItem === 1}
138108
icon={<FolderIcon />}
139109
ref={navItem2Ref}
110+
aria-label="Link 2"
140111
/>
141112
<NavItem
142113
key="nav-icon-link3"
143114
preventDefault
144115
id="nav-icon-link3"
145116
to="#nav-icon-link3"
146-
itemId={0}
117+
itemId={2}
147118
isActive={activeItem === 2}
148119
icon={<CloudIcon />}
149120
ref={navItem3Ref}
121+
aria-label="Link 3"
150122
/>
151123
<NavItem
152124
key="nav-icon-link4"
153125
preventDefault
154126
id="nav-icon-link4"
155127
to="#nav-icon-link4"
156-
itemId={0}
128+
itemId={3}
157129
isActive={activeItem === 3}
158130
icon={<CodeIcon />}
159131
ref={navItem4Ref}
132+
aria-label="Link 4"
160133
/>
161134
</NavList>
162135
</Nav>

‎packages/react-core/src/components/Nav/NavItem.tsx‎

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
useRef,
88
useState,
99
forwardRef,
10-
MutableRefObject
10+
useCallback
1111
} from 'react';
1212
import styles from '@patternfly/react-styles/css/components/Nav/nav';
1313
import menuStyles from '@patternfly/react-styles/css/components/Menu/menu';
@@ -53,7 +53,7 @@ export interface NavItemProps extends Omit<React.HTMLProps<HTMLAnchorElement>, '
5353
/** Set the value of data-ouia-safe. Only set to true when the component is in a static state, i.e. no animations are occurring. At all other times, this value must be false. */
5454
ouiaSafe?: boolean;
5555
/** @hide Forwarded ref */
56-
innerRef?: React.Ref<any>;
56+
innerRef?: React.Ref<HTMLLIElement>;
5757
}
5858

5959
const NavItemBase: React.FunctionComponent<NavItemProps> = ({
@@ -76,13 +76,25 @@ const NavItemBase: React.FunctionComponent<NavItemProps> = ({
7676
innerRef,
7777
...props
7878
}: NavItemProps) => {
79-
const { flyoutRef, setFlyoutRef, navRef } = useContext(NavContext);
79+
const { flyoutRef: contextFlyoutRef, setFlyoutRef, navRef } = useContext(NavContext);
8080
const { isSidebarOpen } = useContext(PageSidebarContext);
8181
const [flyoutTarget, setFlyoutTarget] = useState(null);
8282
const [isHovered, setIsHovered] = useState(false);
83-
const _ref = useRef<HTMLLIElement>(undefined);
84-
const ref = (innerRef as MutableRefObject<HTMLLIElement>) || _ref;
85-
const flyoutVisible = ref === flyoutRef;
83+
const flyoutRef = useRef<HTMLLIElement | null>(null);
84+
const ref = useCallback(
85+
(node: HTMLLIElement | null) => {
86+
flyoutRef.current = node;
87+
if (typeof innerRef === 'function') {
88+
innerRef(node);
89+
} else if (innerRef) {
90+
(innerRef as React.MutableRefObject<HTMLLIElement | null>).current = node;
91+
}
92+
},
93+
[innerRef]
94+
);
95+
const contextFlyoutRefCurrent =
96+
typeof contextFlyoutRef === 'object' && contextFlyoutRef ? contextFlyoutRef.current : null;
97+
const flyoutVisible = flyoutRef?.current === contextFlyoutRefCurrent;
8698
const popperRef = useRef<HTMLDivElement>(undefined);
8799
const hasFlyout = flyout !== undefined;
88100
const Component = hasFlyout ? 'button' : (component as any);
@@ -93,42 +105,48 @@ const NavItemBase: React.FunctionComponent<NavItemProps> = ({
93105
console.error('NavItem cannot have both "to" and "flyout" props.');
94106
}
95107

96-
const showFlyout = (show: boolean, override?: boolean) => {
97-
if ((!flyoutVisible || override) && show) {
98-
setFlyoutRef(ref);
99-
} else if ((flyoutVisible || override) && !show) {
100-
setFlyoutRef(null);
101-
}
108+
const showFlyout = useCallback(
109+
(show: boolean, override?: boolean) => {
110+
if ((!flyoutVisible || override) && show) {
111+
setFlyoutRef(flyoutRef);
112+
} else if ((flyoutVisible || override) && !show) {
113+
setFlyoutRef(null);
114+
}
102115

103-
onShowFlyout && show && onShowFlyout();
104-
};
116+
onShowFlyout && show && onShowFlyout();
117+
},
118+
[flyoutVisible, setFlyoutRef, flyoutRef, onShowFlyout]
119+
);
105120

106121
const onMouseOver = (event: React.MouseEvent) => {
107122
const evtContainedInFlyout = (event.target as HTMLElement).closest(`.${styles.navItem}.pf-m-flyout`);
108123
if (hasFlyout && !flyoutVisible) {
109124
showFlyout(true);
110-
} else if (flyoutRef !== null && !evtContainedInFlyout) {
125+
} else if (contextFlyoutRef !== null && !evtContainedInFlyout) {
111126
setFlyoutRef(null);
112127
}
113128
};
114129

115-
const onFlyoutClick = (event: MouseEvent) => {
116-
const target = event.target;
117-
const closestItem = (target as HTMLElement).closest('.pf-m-flyout');
118-
if (!closestItem) {
119-
if (hasFlyout) {
120-
showFlyout(false, true);
121-
} else if (flyoutRef !== null) {
122-
setFlyoutRef(null);
130+
const onFlyoutClick = useCallback(
131+
(event: MouseEvent) => {
132+
const target = event.target;
133+
const closestItem = (target as HTMLElement).closest('.pf-m-flyout');
134+
if (!closestItem) {
135+
if (hasFlyout) {
136+
showFlyout(false, true);
137+
} else if (contextFlyoutRef !== null) {
138+
setFlyoutRef(null);
139+
}
123140
}
124-
}
125-
};
141+
},
142+
[hasFlyout, showFlyout, contextFlyoutRef, setFlyoutRef]
143+
);
126144

127145
const handleFlyout = (event: KeyboardEvent) => {
128146
const key = event.key;
129147
const target = event.target as HTMLElement;
130148

131-
if ((key === ' ' || key === 'Enter' || key === 'ArrowRight') && hasFlyout && ref?.current?.contains(target)) {
149+
if ((key === ' ' || key === 'Enter' || key === 'ArrowRight') && hasFlyout && flyoutRef?.current?.contains(target)) {
132150
event.stopPropagation();
133151
event.preventDefault();
134152
if (!flyoutVisible) {
@@ -160,7 +178,7 @@ const NavItemBase: React.FunctionComponent<NavItemProps> = ({
160178
window.removeEventListener('click', onFlyoutClick);
161179
}
162180
};
163-
}, []);
181+
}, [hasFlyout, onFlyoutClick]);
164182

165183
useEffect(() => {
166184
if (flyoutTarget) {
@@ -244,7 +262,7 @@ const NavItemBase: React.FunctionComponent<NavItemProps> = ({
244262

245263
const flyoutPopper = (
246264
<Popper
247-
triggerRef={ref}
265+
triggerRef={flyoutRef}
248266
popper={
249267
<div ref={popperRef} onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave}>
250268
{flyout}
@@ -282,8 +300,8 @@ const NavItemBase: React.FunctionComponent<NavItemProps> = ({
282300
return navItem;
283301
};
284302

285-
export const NavItem = forwardRef((props: NavItemProps, ref: React.Ref<any>) => (
286-
<NavItemBase innerRef={ref} {...props} />
303+
export const NavItem = forwardRef<HTMLLIElement, NavItemProps>((props, ref) => (
304+
<NavItemBase {...props} innerRef={ref} />
287305
));
288306

289307
NavItem.displayName = 'NavItem';

‎packages/react-core/src/demos/Page.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import FolderIcon from '@patternfly/react-icons/dist/esm/icons/folder-icon';
1818
import CloudIcon from '@patternfly/react-icons/dist/esm/icons/cloud-icon';
1919
import CodeIcon from '@patternfly/react-icons/dist/esm/icons/code-icon';
2020
import pfLogo from '@patternfly/react-core/src/demos/assets/PF-HorizontalLogo-Color.svg';
21-
import pfIconLogo from '@patternfly/react-core/src/demos/assets/PF-IconLogo-color.svg';
21+
import imgAvatar from '../../../components/assets/avatarImg.svg';
22+
import pfIconLogo from '../../assets/PF-IconLogo-color.svg';
2223

2324
- All examples set the `isManagedSidebar` prop on the Page component to have the sidebar automatically close for smaller screen widths. You can also manually control this behavior by not adding the `isManagedSidebar` prop and instead:
2425

‎packages/react-core/src/demos/examples/Page/PageDockedNav.tsx‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ import CubeIcon from '@patternfly/react-icons/dist/esm/icons/cube-icon';
3939
import FolderIcon from '@patternfly/react-icons/dist/esm/icons/folder-icon';
4040
import CloudIcon from '@patternfly/react-icons/dist/esm/icons/cloud-icon';
4141
import CodeIcon from '@patternfly/react-icons/dist/esm/icons/code-icon';
42-
import imgAvatar from '@patternfly/react-core/src/components/assets/avatarImg.svg';
43-
import pfIconLogo from '@patternfly/react-core/src/demos/assets/PF-IconLogo-color.svg';
44-
42+
import imgAvatar from '../../../components/assets/avatarImg.svg';
43+
import pfIconLogo from '../../assets/PF-IconLogo-color.svg';
4544
interface NavOnSelectProps {
4645
groupId: number | string;
4746
itemId: number | string;
@@ -144,6 +143,7 @@ export const PageDockedNav: React.FunctionComponent = () => {
144143
isActive={activeItem === 0}
145144
icon={<CubeIcon />}
146145
ref={navItem1Ref}
146+
aria-label="Link 1"
147147
/>
148148
<NavItem
149149
preventDefault
@@ -153,24 +153,27 @@ export const PageDockedNav: React.FunctionComponent = () => {
153153
isActive={activeItem === 1}
154154
icon={<FolderIcon />}
155155
ref={navItem2Ref}
156+
aria-label="Link 2"
156157
/>
157158
<NavItem
158159
preventDefault
159-
id="nav-icon-link1"
160-
to="#nav-icon-link1"
161-
itemId={0}
160+
id="nav-icon-link3"
161+
to="#nav-icon-link3"
162+
itemId={2}
162163
isActive={activeItem === 2}
163164
icon={<CloudIcon />}
164165
ref={navItem3Ref}
166+
aria-label="Link 3"
165167
/>
166168
<NavItem
167169
preventDefault
168-
id="nav-icon-link1"
169-
to="#nav-icon-link1"
170-
itemId={0}
170+
id="nav-icon-link4"
171+
to="#nav-icon-link4"
172+
itemId={3}
171173
isActive={activeItem === 3}
172174
icon={<CodeIcon />}
173175
ref={navItem4Ref}
176+
aria-label="Link 4"
174177
/>
175178
</NavList>
176179
</Nav>

0 commit comments

Comments
 (0)