-
Notifications
You must be signed in to change notification settings - Fork 142
Upgrade vue-final-modal to v4 for modal component #2785
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
Upgrade vue-final-modal to v4 for modal component #2785
Conversation
See https://v4.vue-final-modal.org/get-started/guide/migration-guide for migration guide and guidelines driving changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR upgrades vue-final-modal from v3 to v4 in the @markbind/vue-components package, modernizing the modal implementation to follow Vue 3 best practices. The upgrade also fixes UI clipping issues caused by CSS stacking contexts by enforcing teleportation to the body element.
Changes:
- Global configuration updated to use
createVfm()instead ofvfmPluginwith required CSS import - Modal.vue migrated to v4 API with renamed props and explicit teleport configuration
- Trigger.vue refactored to use
useVfm()composition API instead of global$vfm - Tests refactored to use real DOM testing with improved snapshot coverage
Reviewed changes
Copilot reviewed 6 out of 30 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vue-components/src/index.js | Updated global configuration to use createVfm() and imported v4 CSS |
| packages/vue-components/src/Modal.vue | Migrated props to v4 API (modal-id, content-transition, etc.) and added explicit teleport-to |
| packages/vue-components/src/Trigger.vue | Replaced global $vfm with useVfm() composition API |
| packages/vue-components/src/tests/Modal.spec.js | Refactored tests to use real DOM with attachTo: document.body and improved timing |
| packages/vue-components/src/tests/snapshots/Modal.spec.js.snap | Updated snapshots to reflect v4 structure and transitions |
| package.json & package-lock.json | Upgraded vue-final-modal dependency to v4.5.5 with new peer dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2785 +/- ##
==========================================
- Coverage 71.67% 71.67% -0.01%
==========================================
Files 134 134
Lines 7284 7286 +2
Branches 1607 1583 -24
==========================================
+ Hits 5221 5222 +1
- Misses 2017 2018 +1
Partials 46 46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 7 out of 31 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What is the purpose of this pull request?
Overview of changes:
Upgrades
vue-final-modalfrom v3 to v4 in@markbind/vue-componentsto support Vue 3 best practices and future-proof the dependency. Small bonus that v4 is rewritten in TypeScript as well.1. Global Configuration
vfmPluginwithcreateVfm()and added the required CSS import (vue-final-modal/style.css).2. Component Migration
name→modal-id,classes→class,transition→content-transition.:teleport-to="'body'"to ensure correct positioning (it is the default, but added it to highlight it).ssrprop handling.$vfmobject touseVfm()composition API.3. Test Refactoring
attachTo: document.bodyand removed teleport stubs. This ensures tests verify actual content rendering in the DOM.mountModalhelper and improved timing withawait nextTick()to account for v4 transition delays..modalwrapper and rendered content.Refer here for migration guide: https://v4.vue-final-modal.org/get-started/guide/migration-guide
Anything you'd like to highlight/discuss:
A nice touch is that added
overlay-transition="vfm-fade", supported in v4, which is quite subtle, but basically the dimmed overlay background has a slight fade transition compared to before which is immediate.See if u can tell the difference!
Also, this is also to fix the issue found in customviews-js/customviews#111
Bug was CSS Stacking Contexts, where the modal was nested inside a parent element (like
<cv-toggle>) with CSS transforms, browsers clipped the modal. By migrating to v4 and enforcing:teleport-to="'body'", the modal now physically moves to the<body>tag, breaking out of the parent's restrictive stacking context.Basically:
<body>), positioning itself directly relative to the window.Testing instructions:
npm run testProposed commit message: (wrap lines at 72 characters)
Upgrade vue-final-modal to v4 and refactor tests
This upgrade fixes potential UI clipping issues
and modernizes the modal implementation.
Changes:
vue-final-modaldependency to v4.vfmPluginwithcreateVfmand import CSS.modal-id,content-transition) andforce
teleport-to="body"to resolve clipping bugs.useVfmcomposable.Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):