Skip to content

Commit 89ac728

Browse files
micahgodboltspmonahankhmakoto
authored
fix: Removing possible recursive loop in Coachmark (#26934)
* fix possible recursive loop * change * add in additional deps that linting didnt require * fix: refactor useGetBounds Moves useGetBounds() to module scope so the hook is not recreated on every render. This also ensures we're not relying on component scope to consume props. Updates useGetBounds() to use useEffect()'s dependency array to prevent continual re-renders. --------- Co-authored-by: Sean Monahan <seanmonahan@microsoft.com> Co-authored-by: Makoto Morimoto <humbertomakotomorimoto@gmail.com>
1 parent e7adfea commit 89ac728

2 files changed

Lines changed: 22 additions & 12 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix: Coachmark remove possible recursive loop",
4+
"packageName": "@fluentui/react",
5+
"email": "mgodbolt@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

packages/react/src/components/Coachmark/Coachmark.base.tsx

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { FocusTrapZone } from '../../FocusTrapZone';
2323
import { useAsync, useOnEvent, useSetTimeout, useWarnings } from '@fluentui/react-hooks';
2424
import type { IRectangle } from '../../Utilities';
2525
import type { IPositionedData } from '../../Positioning';
26+
import type { IPositioningContainerProps } from './PositioningContainer/PositioningContainer.types';
2627
import type { ICoachmarkProps, ICoachmarkStyles, ICoachmarkStyleProps } from './Coachmark.types';
2728
import type { IBeakProps } from './Beak/Beak.types';
2829

@@ -401,6 +402,15 @@ function useDeprecationWarning(props: ICoachmarkProps) {
401402
}
402403
}
403404

405+
function useGetBounds(props: ICoachmarkProps): IRectangle | undefined {
406+
const async = useAsync();
407+
const [bounds, setBounds] = React.useState<IRectangle | undefined>();
408+
React.useEffect(() => {
409+
async.requestAnimationFrame(() => setBounds(getBounds(props.isPositionForced, props.positioningContainerProps)));
410+
}, [async, props.isPositionForced, props.positioningContainerProps]);
411+
return bounds;
412+
}
413+
404414
const COMPONENT_NAME = 'CoachmarkBase';
405415

406416
export const CoachmarkBase: React.FunctionComponent<ICoachmarkProps> = React.forwardRef<
@@ -467,24 +477,14 @@ export const CoachmarkBase: React.FunctionComponent<ICoachmarkProps> = React.for
467477

468478
const finalHeight: number | undefined = isCollapsed ? COACHMARK_HEIGHT : entityInnerHostRect.height;
469479

470-
function useGetBounds(): IRectangle | undefined {
471-
const async = useAsync();
472-
const [bounds, setBounds] = React.useState<IRectangle | undefined>();
473-
const updateAsyncPosition = (): void => {
474-
async.requestAnimationFrame(() => setBounds(getBounds(props)));
475-
};
476-
React.useEffect(updateAsyncPosition);
477-
return bounds;
478-
}
479-
480480
return (
481481
<PositioningContainer
482482
target={target}
483483
offsetFromTarget={BEAK_HEIGHT}
484484
finalHeight={finalHeight}
485485
ref={forwardedRef}
486486
onPositioned={onPositioned}
487-
bounds={useGetBounds()}
487+
bounds={useGetBounds(props)}
488488
{...positioningContainerProps}
489489
>
490490
<div className={classNames.root}>
@@ -536,7 +536,10 @@ export const CoachmarkBase: React.FunctionComponent<ICoachmarkProps> = React.for
536536
});
537537
CoachmarkBase.displayName = COMPONENT_NAME;
538538

539-
function getBounds({ isPositionForced, positioningContainerProps }: ICoachmarkProps): IRectangle | undefined {
539+
function getBounds(
540+
isPositionForced?: boolean,
541+
positioningContainerProps?: IPositioningContainerProps,
542+
): IRectangle | undefined {
540543
if (isPositionForced) {
541544
// If directionalHint direction is the top or bottom auto edge, then we want to set the left/right bounds
542545
// to the window x-axis to have auto positioning work correctly.

0 commit comments

Comments
 (0)