fix(framer-motion): replace motion with LazyMotion+m to reduce bundle size (~30kb)#10385
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR migrates 10 components across admin and common modules from direct Framer Motion ChangesFramer Motion LazyMotion Bundle Optimization
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
features/admin.branding.v1/components/branding-page-layout.tsx (1)
382-503:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
LazyMotionwrapper breaks animations for branding mode toggle and application dropdown.The
m.divelements at lines 383–434 (branding mode toggle) and 438–501 (application dropdown) are not wrapped inLazyMotion. When using themcomponent factory from Framer Motion, you must wrap animated elements in<LazyMotion features={domAnimation}>to provide animation features. Without this wrapper, them.divelements will render as static containers without animations, breaking the slide-in/fade transitions.🔧 Proposed fix
Wrap the entire
LayoutGroupsection withLazyMotion:<div className="branding-mode-container"> + <LazyMotion features={ domAnimation }> <LayoutGroup> <m.div initial="enter" animate="in" exit="exit" ... </m.div> { brandingMode === BrandingModes.APPLICATION && !appIdFromQueryParam && ( <m.div initial="enter" animate="in" exit="exit" ... </m.div> ) } </LayoutGroup> + </LazyMotion> </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.branding.v1/components/branding-page-layout.tsx` around lines 382 - 503, The animated m.div elements (used for the branding toggle and application dropdown inside LayoutGroup) must be wrapped in Framer Motion's LazyMotion so features are provided; wrap the existing <LayoutGroup>...</LayoutGroup> block with <LazyMotion features={domAnimation}> ... </LazyMotion>, ensure you import { LazyMotion, domAnimation } from "framer-motion", and keep the existing m.div, animationVariants and layout props unchanged so the slide/fade transitions work.features/common.ai.v1/components/ai-banner-tall.tsx (1)
77-136:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing LazyMotion wrapper breaks animations.
The
m.divelement on line 78 will not animate because there is no<LazyMotion features={domAnimation}>wrapper to provide the animation features. Unlike the other migrated files in this PR, this component is missing the required provider.🔧 Proposed fix
return ( + <LazyMotion features={ domAnimation }> <m.div className="ai-banner-motion-container" style={ { backgroundImage: `url(${ AIBot }), url(${ AIBannerBackgroundTall })` } } animate={ showContent ? "expanded" : "collapsed" } variants={ variants } transition={ transitions } > <Accordion className="ai-banner-container" defaultExpanded disableGutters elevation={ 0 } sx={ { "&:before": { display: "none" } } } onChange={ () => setShowContent(!showContent) } > <AccordionSummary className="ai-banner-content" expandIcon={ ( <IconButton className="ai-banner-collapse-button" > <ChevronDownIcon className="ai-banner-caret-icon" size={ 14 } fill="black" /> </IconButton> ) } > <Box className="ai-banner-text-container"> <Typography component="h3" className="ai-banner-heading" > { title } <AIText> { aiText } </AIText> { titleLabel } </Typography> <Typography className="ai-banner-sub-heading"> { description } </Typography> </Box> </AccordionSummary> <Box className="ai-banner-children"> { children } </Box> </Accordion> </m.div> + </LazyMotion> );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/common.ai.v1/components/ai-banner-tall.tsx` around lines 77 - 136, The m.div motion container in AI-banner-tall.tsx won't animate because it lacks the Framer Motion feature provider; wrap the returned m.div with <LazyMotion features={domAnimation}> (import LazyMotion and domAnimation from "framer-motion") so the m.div can access animation features, and ensure the imports for LazyMotion and domAnimation are added to the file alongside the existing m import.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/thirty-zebras-tap.md:
- Line 5: The component is misspelled as ModelWithSidePanel; rename the
component symbol to ModalWithSidePanel where it is declared/exported (e.g.,
change the React component/function/class name and default/named export for
ModelWithSidePanel) and update all imports/usages to reference
ModalWithSidePanel instead; also rename any files or index re-exports that use
"ModelWithSidePanel" in their filename or export names so everything
(declaration, exports, and consumers) consistently uses ModalWithSidePanel.
In `@features/common.ai.v1/components/ai-loading-screen.tsx`:
- Line 78: The m.div wrapper uses animate="botAnimation" but no botAnimation
variant is defined in this component (only factVariants exists) and botAnimation
belongs to the child AIBotAnimatedWithBackGround; fix by removing
animate="botAnimation" from the m.div (or alternatively define a local Variants
object named botAnimation and pass it to the m.div), and ensure any animation
props reference factVariants or the correctly defined botAnimation variant so
m.div's animate prop matches a variant declared in this component.
---
Outside diff comments:
In `@features/admin.branding.v1/components/branding-page-layout.tsx`:
- Around line 382-503: The animated m.div elements (used for the branding toggle
and application dropdown inside LayoutGroup) must be wrapped in Framer Motion's
LazyMotion so features are provided; wrap the existing
<LayoutGroup>...</LayoutGroup> block with <LazyMotion features={domAnimation}>
... </LazyMotion>, ensure you import { LazyMotion, domAnimation } from
"framer-motion", and keep the existing m.div, animationVariants and layout props
unchanged so the slide/fade transitions work.
In `@features/common.ai.v1/components/ai-banner-tall.tsx`:
- Around line 77-136: The m.div motion container in AI-banner-tall.tsx won't
animate because it lacks the Framer Motion feature provider; wrap the returned
m.div with <LazyMotion features={domAnimation}> (import LazyMotion and
domAnimation from "framer-motion") so the m.div can access animation features,
and ensure the imports for LazyMotion and domAnimation are added to the file
alongside the existing m import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ac988988-fd68-45dc-9e9c-535f75eb86f0
📒 Files selected for processing (18)
.changeset/thirty-zebras-tap.mdfeatures/admin.agents.v1/components/edit/share-agent-form.tsxfeatures/admin.applications.v1/components/forms/share-application-form-updated.tsxfeatures/admin.applications.v1/components/modals/application-share-modal-updated.tsxfeatures/admin.branding.v1/components/branding-page-layout.tsxfeatures/admin.console-settings.v1/components/console-shared-access/console-shared-access.tsxfeatures/admin.core.v1/components/modals/modal-with-side-panel-actions.tsxfeatures/admin.core.v1/components/modals/modal-with-side-panel-content.tsxfeatures/admin.core.v1/components/modals/modal-with-side-panel-header.tsxfeatures/admin.core.v1/components/modals/modal-with-side-panel-main-panel.tsxfeatures/admin.core.v1/components/modals/modal-with-side-panel-side-panel.tsxfeatures/admin.core.v1/components/modals/modal-with-side-panel-types.tsfeatures/admin.core.v1/components/modals/modal-with-side-panel.tsxfeatures/admin.home.v1/components/new-feature-announcement/new-feature-announcement.tsxfeatures/admin.users.v1/components/share-user-form.tsxfeatures/common.ai.v1/components/ai-banner-tall.tsxfeatures/common.ai.v1/components/ai-bot-animated-with-bg.tsxfeatures/common.ai.v1/components/ai-loading-screen.tsx
|
Hi @samsameer2804-cloud could you please add a screen recording of how your fix works? Also, please add the screenshots for the before and after bundle analysis as well. Thanks! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10385 +/- ##
=======================================
Coverage 98.79% 98.79%
=======================================
Files 165 165
Lines 51764 51764
Branches 111 111
=======================================
Hits 51138 51138
Misses 626 626
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…ents to save ~30kb bundle size
… botAnimation variant
samsameer2804-cloud
force-pushed
the
fix/use-lazy-motion-framer-motion
branch
from
d7a0513 to
891958e
Compare
Hi @pavinduLakshan, unfortunately I'm unable to run the full WSO2 stack locally on Windows to produce a screen recording or bundle analysis. This is a pure refactoring change with no behavioral difference — the framer-motion docs confirm the ~30kb saving: https://www.framer.com/motion/lazy-motion/. Happy to help in any other way! |
Thanks for the contribution. Unfortunately, we can’t accept changes that haven’t been verified by running the product locally. While this PR is primarily a refactoring, seemingly harmless refactors can sometimes introduce unexpected issues or edge cases. I've personally seen many such changes lead to regressions for reasons that weren't apparent during code review. As a result, we require contributors to validate their changes locally and provide evidence of that verification before we can merge a PR. Thanks for your understanding. |
|
Hi @pavinduLakshan, Let me know if any further changes are needed! WSO2.Identity.Server.-.Profile.1.-.Microsoft.Edge.2026-06-13.15-35-42.mp4
|
|
Hi @pavinduLakshan and reviewers, Added missing LazyMotion wrapper in branding-page-layout.tsx for the branding mode toggle and application dropdown animations I ran the project locally (WSO2 IS 7.3.0 + console dev server) and verified the changes. Attached is a screen recording showing navigation across Home and Branding pages with animations rendering correctly, and a console screenshot confirming no framer-motion/LazyMotion related errors (remaining 404s are pre-existing backend resource issues unrelated to this PR). |
The screen recording doesn't show the console running from the local dev server. Instead it looks like the console is running from the WSO2 Identity server itself. Could you please check again? And the screen recording doesn't show the branding view. Can u capture that as well to your screen recording? PS: Also please make sure to add the bundle size comparison evidence as well to the pr description |
Hi @pavinduLakshan, Thank you for the detailed feedback. I've re-recorded the verification on the local Vite dev server (port 9001) as requested. What was done to set this up locally:
What the recording shows:
Bundle size evidence:
Reference: Framer Motion LazyMotion docs — lint rule: Ready for re-review! Home.and.1.more.page.-.Profile.1.-.Microsoft_.Edge.2026-06-15.17-43-05.mp4
|
2 participants
Purpose
Fixes the
react-doctor/use-lazy-motionviolation by replacingmotionimports with
LazyMotion+mpattern across 10 components.Related Issues
Closes wso2/product-is#27961
Changes
admin.agents.v1/components/edit/share-agent-form.tsxadmin.applications.v1/components/forms/share-application-form-updated.tsxadmin.applications.v1/components/modals/application-share-modal-updated.tsxadmin.branding.v1/components/branding-page-layout.tsxadmin.console-settings.v1/components/console-shared-access/console-shared-access.tsxadmin.home.v1/components/new-feature-announcement/new-feature-announcement.tsxadmin.users.v1/components/share-user-form.tsxcommon.ai.v1/components/ai-bot-animated-with-bg.tsxcommon.ai.v1/components/ai-banner-tall.tsxcommon.ai.v1/components/ai-loading-screen.tsxChecklist
Testing