refactor(Popover, Menu): migrate slide animation from CSS to motion component#35763
Open
robertpenner wants to merge 27 commits intomicrosoft:masterfrom
Open
refactor(Popover, Menu): migrate slide animation from CSS to motion component#35763robertpenner wants to merge 27 commits intomicrosoft:masterfrom
robertpenner wants to merge 27 commits intomicrosoft:masterfrom
Conversation
robertpenner
force-pushed
the
feat/popover-motion
branch
2 times, most recently
from
25328c8 to
3ca9a82
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
packages/react-components/react-popover/library/src/components/Popover/Popover.types.ts
Show resolved
Hide resolved
robertpenner
force-pushed
the
feat/popover-motion
branch
3 times, most recently
from
fb9e16d to
eb19b8a
Compare
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
robertpenner
force-pushed
the
feat/popover-motion
branch
from
eb19b8a to
17c8fce
Compare
robertpenner
changed the title
robertpenner
force-pushed
the
feat/popover-motion
branch
from
71af618 to
7198c44
Compare
robertpenner
force-pushed
the
feat/popover-motion
branch
from
350a125 to
4256914
Compare
…ation - Add surfaceMotion presence motion slot to PopoverProps/PopoverState - Create PopoverSurfaceMotion using fadeAtom + slideAtom with CSS custom property-based direction - Create usePositioningSlideDirection hook to set slide direction CSS vars from placement - Wire MotionRefForwarder into renderPopover for ref forwarding through motion wrapper - Add useMotionForwardedRef to PopoverSurface for motion ref consumption - Remove old createSlideStyles usage from PopoverSurface styles - Add react-motion and react-motion-components-preview dependencies - Add unit tests for usePositioningSlideDirection and getPlacementSlideDirections
Slide animations are now handled by PopoverSurfaceMotion in react-popover.
…les function with explanation
Replace deprecated createSlideStyles with the surfaceMotion slot pattern, matching the approach used in react-popover. This uses fadeAtom + slideAtom motion components driven by CSS custom properties set from positioning placement data via onPositioningEnd. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ce motion components
… null
PresenceMotionSlotProps does not include null in its type, so passing
surfaceMotion={null} to disable motion was rejected by TypeScript. Wrapping
it in Slot<> adds null to the union, matching the pattern already used by
Drawer's motion slots.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Demonstrates disabling the Popover transition animation by passing
surfaceMotion={null}.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MotionCustom demonstrates replacing the default slide animation with a
custom fade-in/blur-out motion using createPresenceComponent and motion
atoms. MotionDisabled demonstrates passing surfaceMotion={null} to disable
the transition entirely.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fix JSX type error
The custom @fluentui/react-jsx-runtime jsx function uses React.ElementType<Props>
which does not include ForwardRefExoticComponent. TypeScript falls back to Props={}
and then requires the explicit JSX attributes to satisfy all required props, causing
TS2741 when children is required but only provided as JSX children (not an attribute).
Making children optional resolves the false positive across react-menu, react-popover,
react-dialog, and react-message-bar without changing runtime behaviour.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… files The @jsxImportSource @fluentui/react-jsx-runtime pragma is only needed when Slot API components are used in JSX. After switching to MotionRefForwarder these render files no longer use Slot, so the pragma (and the accompanying @jsxRuntime automatic directive) are removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…deDirection tests Replace `as any` casts with a proper SlideDirectionEvent type alias derived from PositioningProps['onPositioningEnd'], using variable annotations to let TypeScript infer the CustomEvent generic via contextual typing. Replace mockWindow `as any` with `as unknown as Window & typeof globalThis`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…aceBase React Compiler flags mutation of values returned from hooks. Instead of assigning to state.root.ref after the fact, hoist useMotionForwardedRef() and pass the pre-merged ref directly into usePopoverSurfaceBase_unstable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… into react-positioning Move `usePositioningSlideDirection` and `getPlacementSlideDirections` from their duplicate local copies in `react-menu` and `react-popover` into `react-positioning`, where the positioning concerns belong. Also centralise the `--fui-positioning-slide-direction-*` CSS custom property names as exported constants (`POSITIONING_SLIDE_DIRECTION_VAR_X/Y`). Consumers (`useMenu`, `usePopover`, `MenuSurfaceMotion`, `PopoverSurfaceMotion`) now import from `@fluentui/react-positioning` instead of maintaining local copies. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
<state.surfaceMotion> is a SlotComponentType created by presenceMotionSlot() that requires @fluentui/react-jsx-runtime to resolve SLOT_ELEMENT_TYPE_SYMBOL. The pragma was removed in b22f3b0, causing surfaceMotion to be passed as a plain object to React.createElement, which breaks rendering. assertSlots<MenuSlots>(state) is a runtime no-op (MenuSlots is empty), but satisfies the @nx/workspace-no-missing-jsx-pragma lint rule which requires both <state.xxx> JSX usage and an assertSlots() call to consider the pragma as needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…opover Same fix as renderMenu: <state.surfaceMotion> requires @fluentui/react-jsx-runtime to resolve the SlotComponentType from presenceMotionSlot(). Adds PopoverSlots (empty) and components to PopoverState to support the assertSlots() call, which iterates Object.keys(state.components) at runtime in dev mode. PopoverBaseState omits components since the base hook does not need it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…de direction variables
…opover unmount The surfaceMotion presence component keeps the popover mounted briefly during its exit lifecycle, so focus may still be inside the popover when `open` becomes false. Expand the focus-restoration check to also cover that case, ensuring the trigger receives focus before the DOM element is removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
robertpenner
force-pushed
the
feat/popover-motion
branch
from
6e64602 to
04b1f34
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This replaces the CSS-based slide animation in Popover and Menu with a new
surfaceMotionpresence motion slot, powered by theSlideFluent motion component by default.Motivation
The existing
createSlideStylesapproach inreact-positioningembeds animation logic into static Griffel styles, which limits flexibility:The
surfaceMotionslot makes animation a first-class composable concern that consumers can customize, replace with a custom motion component, or disable entirely by passingsurfaceMotion={null}.Changes
react-popover
surfaceMotionpresence motion slot toPopoverProps/PopoverStatePopoverSurfaceMotion— enter-only animation combiningfadeAtom+slideAtomwith CSS custom property-based directionusePositioningSlideDirectionhook — sets slide direction CSS vars fromonPositioningEndplacement data and registers them viaCSS.registerPropertyfor smooth interpolationMotionRefForwarderintorenderPopoverfor ref forwarding through the motion wrapperuseMotionForwardedReftoPopoverSurfacefor motion ref consumptioncreateSlideStylesusage fromPopoverSurfacestylessurfaceMotion={null})usePositioningSlideDirectionandgetPlacementSlideDirectionsreact-menu
surfaceMotionslot,MenuSurfaceMotion, andusePositioningSlideDirectioncreateSlideStylesusage fromMenuPopoverstylesreact-positioning
createSlideStyles— slide animations are now handled by the surface motion components in each packageAPI Surface
New prop on both
PopoverPropsandMenuProps:Wrapping in
Slot<>allowssurfaceMotion={null}to disable animation, matching the pattern used by Drawer motion slots.Backwards Compatibility
surfaceMotionslot is optional and defaults to the built-in slide animation, preserving existing visual behavior.createSlideStylesis deprecated but not removed; existing consumers are unaffected.Testing
getPlacementSlideDirectionscovering all 4 sides (top/right/bottom/left)usePositioningSlideDirectioncovering CSS property registration, style setting per placement, and user callback forwardingPrerequisites