Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popover: refactor to TypeScript #43823

Merged
merged 49 commits into from
Sep 6, 2022
Merged
Changes from 1 commit
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
867c532
Rename files
ciampo Sep 2, 2022
a911e6a
First iteration of Popover props
ciampo Sep 2, 2022
ddfe57b
Add types to component and default export
ciampo Sep 2, 2022
7f4059b
Type AnimatedWrapper, refactor logic to always render a `motion.div`
ciampo Sep 2, 2022
ed5ace4
Remove unnused props from `ArrowTriangle`
ciampo Sep 2, 2022
e2f18b7
Use intermediate variable for flip/resize fallback values when `__uns…
ciampo Sep 2, 2022
742c830
Type PopoverSlot
ciampo Sep 2, 2022
0fd0d43
Type resize middleware
ciampo Sep 2, 2022
c095b2f
Type onDialogClose & related
ciampo Sep 2, 2022
b984ff7
Type middleware array
ciampo Sep 2, 2022
6263832
Split and export different anchor ref types
ciampo Sep 2, 2022
702f95c
Type ownerDocument ev listeners useEffect
ciampo Sep 2, 2022
d9cde6d
Type internal state and ref callbacks
ciampo Sep 2, 2022
9ff0b8b
Type mergedFloatingRef
ciampo Sep 2, 2022
67afd28
Fix type error in AnimatedWrapper s style
ciampo Sep 2, 2022
eccc7ca
Type arrow styles
ciampo Sep 2, 2022
0c8c527
Simplify position pro definition
ciampo Sep 2, 2022
a7d16d3
Type placement-based utils
ciampo Sep 2, 2022
39eb435
Type getFrameOffset
ciampo Sep 2, 2022
6e46741
Type getReferenceOwnerDocument
ciampo Sep 2, 2022
b2698a0
Make top and bottom props on anchorRef mandatory
ciampo Sep 2, 2022
62555b5
Type getReferenceElement
ciampo Sep 2, 2022
4b9aa59
anchorRef.startContainer is never undefined
ciampo Sep 2, 2022
3d60bf9
No need to optional-chain anchorRef.current.ownerDocument
ciampo Sep 2, 2022
79c0918
Remove TODO
ciampo Sep 2, 2022
7b82718
Fix getFrameOffset signature
ciampo Sep 2, 2022
5feac47
Simplify anchorRef types
ciampo Sep 4, 2022
0d69c6e
Add type descriptions
ciampo Sep 4, 2022
448a8b0
Type `useDialog` hook
ciampo Sep 4, 2022
248522b
Add @deprecated tag for the range prop
ciampo Sep 4, 2022
f885dd3
Type Storybook examples
ciampo Sep 4, 2022
64b218f
refactor BorderControl and BorderBoxControl
ciampo Sep 4, 2022
3102956
Add JSDoc description and example snippet to exported component
ciampo Sep 4, 2022
92c96e5
Add `placement` s default value
ciampo Sep 4, 2022
b571090
README
ciampo Sep 4, 2022
bf068e9
CHANGELOGs
ciampo Sep 5, 2022
969bd60
Refine `position` types
ciampo Sep 5, 2022
dec13b2
Type the `shift` prop, mark `__unstableShift` as deprecated
ciampo Sep 5, 2022
fabedfa
focusOnMount typo
ciampo Sep 6, 2022
031fea4
Fix onFocusOutside README type
ciampo Sep 6, 2022
bf84afb
fix getAnchorRect README type
ciampo Sep 6, 2022
6c24c7b
Fix anchorRef README type
ciampo Sep 6, 2022
8261fdf
Fix children README type
ciampo Sep 6, 2022
e4807e0
Fix isAlternate README type
ciampo Sep 6, 2022
27875f8
Remove unstable props from README, add deprecated range prop
ciampo Sep 6, 2022
5754ee1
Improve focusOnMount types on derived components
ciampo Sep 6, 2022
25d4391
Do not display controls for `children` prop
ciampo Sep 6, 2022
78a25e8
Storybook feedback
ciampo Sep 6, 2022
dac81ea
Restore they way popoverProps was passed in BorderControl
ciampo Sep 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Type AnimatedWrapper, refactor logic to always render a motion.div
  • Loading branch information
ciampo committed Sep 6, 2022
commit 7f4059b0bb7205b93edf480f2830d7e0d7b61a48
59 changes: 32 additions & 27 deletions packages/components/src/popover/index.tsx
Original file line number Diff line number Diff line change
@@ -14,7 +14,12 @@ import {
size,
} from '@floating-ui/react-dom';
// eslint-disable-next-line no-restricted-imports
import { motion, useReducedMotion, MotionProps } from 'framer-motion';
import {
motion,
useReducedMotion,
HTMLMotionProps,
MotionProps,
} from 'framer-motion';

/**
* WordPress dependencies
@@ -53,7 +58,7 @@ import {
getReferenceElement,
} from './utils';
import type { WordPressComponentProps } from '../ui/context';
import type { PopoverProps } from './types';
import type { PopoverProps, AnimatedWrapperProps } from './types';

/**
* Name of slot in which popover should fill.
@@ -86,15 +91,15 @@ const ArrowTriangle = ( props ) => (
</SVG>
);

const MaybeAnimatedWrapper = forwardRef(
const AnimatedWrapper = forwardRef(
(
{
style: receivedInlineStyles,
placement,
shouldAnimate = false,
...props
},
forwardedRef
}: HTMLMotionProps< 'div' > & AnimatedWrapperProps,
forwardedRef: ForwardedRef< any >
) => {
// When animating, animate only once (i.e. when the popover is opened), and
// do not animate on subsequent prop changes (as it conflicts with
@@ -112,27 +117,27 @@ const MaybeAnimatedWrapper = forwardRef(
[]
);

if ( shouldAnimate && ! shouldReduceMotion ) {
return (
<motion.div
style={ {
...motionInlineStyles,
...receivedInlineStyles,
} }
{ ...otherMotionProps }
onAnimationComplete={ onAnimationComplete }
animate={
hasAnimatedOnce ? false : otherMotionProps.animate
}
{ ...props }
ref={ forwardedRef }
/>
);
}
const computedAnimationProps: HTMLMotionProps< 'div' > =
shouldAnimate && ! shouldReduceMotion
? {
style: {
...motionInlineStyles,
...receivedInlineStyles,
},
...otherMotionProps,
onAnimationComplete,
animate: hasAnimatedOnce
? false
: otherMotionProps.animate,
}
: {
animate: false,
style: receivedInlineStyles,
};

return (
<div
style={ receivedInlineStyles }
<motion.div
{ ...computedAnimationProps }
Comment on lines +125 to +145
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Runtime change: always return a motion.div — I made this change because it would have been otherwise difficult to type AnimatedWrapper, since standard HTML elements and motion elements have different types for the same prop names

{ ...props }
ref={ forwardedRef }
/>
@@ -427,7 +432,7 @@ const UnforwardedPopover = (
let content = (
// eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<MaybeAnimatedWrapper
<AnimatedWrapper
shouldAnimate={ animate && ! isExpanded }
placement={ computedPlacement }
className={ classnames( 'components-popover', className, {
@@ -437,7 +442,7 @@ const UnforwardedPopover = (
{ ...contentProps }
ref={ mergedFloatingRef }
{ ...dialogProps }
tabIndex="-1"
tabIndex={ -1 }
style={
isExpanded
? undefined
@@ -488,7 +493,7 @@ const UnforwardedPopover = (
<ArrowTriangle />
</div>
) }
</MaybeAnimatedWrapper>
</AnimatedWrapper>
);

if ( slot.ref ) {
5 changes: 5 additions & 0 deletions packages/components/src/popover/types.ts
Original file line number Diff line number Diff line change
@@ -16,6 +16,11 @@ type DomRectWithOwnerDocument = DOMRect & {
ownerDocument?: Document;
};

export type AnimatedWrapperProps = {
placement: Placement;
shouldAnimate?: boolean;
};

// TODO:
// - add all props
// - add deprecations, default values, descriptions