Skip to content

Conversation

@gerteck
Copy link
Member

@gerteck gerteck commented Jan 18, 2026

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:

Upgrades vue-final-modal from v3 to v4 in @markbind/vue-components to support Vue 3 best practices and future-proof the dependency. Small bonus that v4 is rewritten in TypeScript as well.

1. Global Configuration

  • Replaced vfmPlugin with createVfm() and added the required CSS import (vue-final-modal/style.css).

2. Component Migration

  • Modal.vue:
    • Migrated props to v4 API: namemodal-id, classesclass, transitioncontent-transition.
    • Explicitly set :teleport-to="'body'" to ensure correct positioning (it is the default, but added it to highlight it).
    • Removed deprecated ssr prop handling.
  • Trigger.vue:
    • Migrated from global $vfm object to useVfm() composition API.

3. Test Refactoring

  • Real DOM Testing: Refactored tests to use attachTo: document.body and removed teleport stubs. This ensures tests verify actual content rendering in the DOM.
  • Stability: Added mountModal helper and improved timing with await nextTick() to account for v4 transition delays.
  • Snapshots: Updated to capture full .modal wrapper 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:

  • Before: v3 Default: In-Place - The modal was created inside the component code, subject to the parent's CSS rules, and more bug prone.
  • After: v4 uses Vue Teleports, which "teleports" the actual HTML to specified location in the DOM (specifically, the <body>), positioning itself directly relative to the window.

Testing instructions:

npm run test

Proposed 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:

  • Upgrade vue-final-modal dependency to v4.
  • Global: Replace vfmPlugin with createVfm and import CSS.
  • Modal.vue: Rename props (modal-id, content-transition) and
    force teleport-to="body" to resolve clipping bugs.
  • Trigger.vue: Migrate to useVfm composable.
  • Tests: Refactor and update snapshots.

Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

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):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

Copy link

Copilot AI left a 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 of vfmPlugin with 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
Copy link

codecov bot commented Jan 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.67%. Comparing base (277bab1) to head (05358d3).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gerteck gerteck changed the title Chore/upgrade vue final modal v4 Upgrade vue-final-modal to v4 for modal component Jan 18, 2026
@gerteck gerteck requested a review from Copilot January 18, 2026 18:35
Copy link

Copilot AI left a 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.

@gerteck gerteck merged commit a12f57a into MarkBind:master Jan 19, 2026
8 checks passed
@github-actions github-actions bot added the r.Patch Version resolver: increment by 0.0.1 label Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

r.Patch Version resolver: increment by 0.0.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant