From ebf3977e2fc8767e169f22910681470c94d1f113 Mon Sep 17 00:00:00 2001 From: bjorn Date: Wed, 29 Jul 2026 16:36:29 +0200 Subject: [PATCH 1/2] feat: separate show/hide animation durations and optional silent re-open Adds three backwards compatible options to `animation`: - `showDuration` and `hideDuration` override `duration` for a single direction. Both default to null, so a config that only sets `duration` keeps using it for the show as well as the hide animation. - `animateOnReopen` (default true, i.e. current behaviour) controls whether the show animation is replayed when the menu that is being opened is already on screen, for instance when right clicking another element that shares the same menu. With it disabled the menu simply moves to the new position. Re-opening is detected on the menu element itself rather than on "some menu is open", so a different menu opening while another one closes is still animated. When the modal layer is used, opening another trigger waits for the hide animation to finish before showing the menu again, which is exactly what makes the animation replay. That wait is skipped when the same menu is re-opened with `animateOnReopen: false`, so the still visible menu is just repositioned. --- documentation/_data/nav.js | 1 + documentation/demo.md | 1 + documentation/demo/animation.md | 57 ++++++++ documentation/docs.md | 30 ++++- src/jquery.contextMenu.js | 93 ++++++++++++- test/specs/animation-reopen.js | 57 ++++++++ test/unit/issue-739-animation.test.js | 182 ++++++++++++++++++++++++++ 7 files changed, 414 insertions(+), 7 deletions(-) create mode 100644 documentation/demo/animation.md create mode 100644 test/specs/animation-reopen.js create mode 100644 test/unit/issue-739-animation.test.js diff --git a/documentation/_data/nav.js b/documentation/_data/nav.js index 8d7db221..a1b635c6 100644 --- a/documentation/_data/nav.js +++ b/documentation/_data/nav.js @@ -33,6 +33,7 @@ module.exports = [ { id: 'simple-context-menu', text: 'Simple Context Menu', url: '/demo.html' }, { id: 'fontawesome-icons', text: 'FontAwesome icons', url: '/demo/fontawesome-icons.html' }, { id: 'accesskeys', text: 'Accesskeys', url: '/demo/accesskeys.html' }, + { id: 'animation', text: 'Animation Options', url: '/demo/animation.html' }, { id: 'async-create', text: 'Create Context Menu (asynchronous)', url: '/demo/async-create.html' }, { id: 'async-promise', text: 'Create Context Menu (promise)', url: '/demo/async-promise.html' }, { id: 'callback', text: "Command's action (callbacks)", url: '/demo/callback.html' }, diff --git a/documentation/demo.md b/documentation/demo.md index f4ad7e27..c3bca269 100644 --- a/documentation/demo.md +++ b/documentation/demo.md @@ -69,6 +69,7 @@ title: jQuery contextMenu — Demo gallery * [Disabled Callback Command](demo/disabled-callback.html) * [Changing Command's disabled status](demo/disabled-changing.html) * [Accesskeys](demo/accesskeys.html) +* [Animation Options](demo/animation.html) * [Submenus](demo/sub-menus.html) * [Input Commands](demo/input.html) * [Custom Command Types](demo/custom-command.html) diff --git a/documentation/demo/animation.md b/documentation/demo/animation.md new file mode 100644 index 00000000..436197e2 --- /dev/null +++ b/documentation/demo/animation.md @@ -0,0 +1,57 @@ +--- +currentMenu: animation +--- + +# Demo: Animation Options + + + + + +- [Example code](#example-code) +- [Example HTML](#example-html) + + + +Both elements below share the same menu. It fades in slowly and out quickly, and because +`animateOnReopen` is `false` it simply moves to the new position when you right click the +other element while the menu is still open. + +right click me +and then right click me + +## Example code + + + +## Example HTML + diff --git a/documentation/docs.md b/documentation/docs.md index 1aab5662..82d23ecd 100644 --- a/documentation/docs.md +++ b/documentation/docs.md @@ -318,13 +318,39 @@ $.contextMenu({ Animation properties take effect on showing and hiding the menu. Duration specifies the duration of the animation in milliseconds. `show` and `hide` specify [jQuery methods](http://api.jquery.com/category/effects/) to show and hide elements. -`animation`: `object` default: `{duration: 500, show: 'slideDown', hide: 'slideUp'}` +`animation`: `object` default: `{duration: 50, showDuration: null, hideDuration: null, animateOnReopen: true, show: 'slideDown', hide: 'slideUp'}` + +Value | Description +---- | ---- +`animation.duration` | Duration in milliseconds, used for both the show and the hide animation +`animation.showDuration` | Optional duration for the show animation only. Falls back to `animation.duration` when `null` +`animation.hideDuration` | Optional duration for the hide animation only. Falls back to `animation.duration` when `null` +`animation.animateOnReopen` | Whether the show animation is replayed when the menu that is opened is already visible. Set to `false` to only move it +`animation.show` | [jQuery method](http://api.jquery.com/category/effects/) used to show the menu +`animation.hide` | [jQuery method](http://api.jquery.com/category/effects/) used to hide the menu + +`animateOnReopen` applies to that one menu only. Opening the same menu again, for example by right-clicking another element that shares it, moves the menu to the new position without replaying the show animation when it is set to `false`. A different menu opening while another one is closing is always animated. #### Example ```javascript $.contextMenu({ selector: 'span.context-menu', - animation: `{duration: 250, show: 'fadeIn', hide: 'fadeOut'}` + animation: {duration: 250, show: 'fadeIn', hide: 'fadeOut'} +}); +``` + +```javascript +$.contextMenu({ + selector: 'span.context-menu', + animation: { + // fade in slowly, but disappear quickly + showDuration: 400, + hideDuration: 100, + show: 'fadeIn', + hide: 'fadeOut', + // don't animate again when the menu is already on screen + animateOnReopen: false + } }); ``` diff --git a/src/jquery.contextMenu.js b/src/jquery.contextMenu.js index 03d0b7b0..88961c14 100644 --- a/src/jquery.contextMenu.js +++ b/src/jquery.contextMenu.js @@ -397,7 +397,19 @@ zIndex: 1, // show hide animation settings animation: { + // duration used for both the show and the hide animation, + // unless overridden by showDuration/hideDuration below duration: 50, + // optional per-direction durations. When null (the default) + // `duration` is used, so configs that only set `duration` + // keep behaving exactly as before. + showDuration: null, + hideDuration: null, + // whether the show animation is replayed when the very same + // menu is already on screen and gets re-opened (e.g. by + // right-clicking another trigger sharing that menu). Set to + // false to just move the menu instead of animating it again. + animateOnReopen: true, show: 'slideDown', hide: 'slideUp' }, @@ -760,10 +772,22 @@ } } + var openTargetMenu; if (target && triggerAction) { - root.$trigger.one('contextmenu:hidden', function () { + openTargetMenu = function () { $(target).contextMenu({x: x, y: y, button: button}); - }); + }; + + // Re-opening the very same menu on another trigger normally + // waits for the hide animation to finish before showing it + // again, which is what makes the menu collapse and expand + // again. With animation.animateOnReopen disabled, show it + // right away instead, so op.show() just moves the menu that + // is still on screen (see #739). + if (!reopensSameMenuWithoutAnimation(root, target)) { + root.$trigger.one('contextmenu:hidden', openTargetMenu); + openTargetMenu = null; + } } // See the comment above isNearRecentSelectChange() / @@ -776,6 +800,10 @@ // select's own horizontal span. if (root !== null && typeof root !== 'undefined' && root.$menu !== null && typeof root.$menu !== 'undefined' && !isNearRecentSelectChange(root, x, y)) { root.$menu.trigger('contextmenu:hide'); + + if (openTargetMenu) { + openTargetMenu(); + } } }, 50); }, @@ -1282,7 +1310,16 @@ op = { show: function (opt, x, y) { var $trigger = $(this), - css = {}; + css = {}, + // Whether this very menu element is already on screen, i.e. + // it is being re-opened (and repositioned) rather than shown + // for the first time. This has to be sampled before any open + // menu is hidden below, because that hide only *starts* an + // animation - the menu stays visible while it runs. + // Deliberately checks opt.$menu itself instead of "is any + // menu open": another menu closing and this one opening is a + // genuine show, not a re-open. + isReopen = !!(opt.$menu && opt.$menu.length && opt.$menu.is(':visible')); // hide any open menus if ($('#context-menu-layer').length > 0) @@ -1326,7 +1363,24 @@ opt.$menu.find('ul').css('zIndex', css.zIndex + 1); // position and show context menu - opt.$menu.css(css)[opt.animation.show](opt.animation.duration, function () { + var $menu = opt.$menu.css(css), + showDuration = animationDuration(opt.animation, 'showDuration'); + + if (isReopen && opt.animation.animateOnReopen === false) { + // The menu is already on screen and merely moves to a new + // position, so don't replay the show animation. Any hide + // animation started by the re-open is jumped to its end + // (running its completion callbacks) and the menu is put + // back into its shown state synchronously, so it never + // gets a chance to be painted as hidden. The show effect + // is still invoked - on an already visible element it is a + // no-op apart from its callback - so custom effects keep + // being called exactly once per show. + $menu.stop(true, true).show(); + showDuration = 0; + } + + $menu[opt.animation.show](showDuration, function () { $trigger.trigger('contextmenu:visible'); var rootShowTimestamp = Date.now(); @@ -1414,7 +1468,7 @@ $(document).off('.contextMenuAutoHide').off('keydown.contextMenu'); // hide menu if (opt.$menu) { - opt.$menu[opt.animation.hide](opt.animation.duration, function () { + opt.$menu[opt.animation.hide](animationDuration(opt.animation, 'hideDuration'), function () { // tear down dynamically built menu after animation is completed. if (opt.build) { opt.$menu.remove(); @@ -2182,6 +2236,35 @@ } }; + // true if opening the menu on `target` puts the very same menu that `root` + // is currently showing back on screen, and that menu is configured not to + // animate such a re-open (animation.animateOnReopen: false) + function reopensSameMenuWithoutAnimation(root, target) { + // menus built on invocation get a brand new menu element every time, so + // they are never re-opened - they're torn down and built up again, which + // has to wait for the hide to finish + if (!root || root.build || !root.animation || root.animation.animateOnReopen !== false || !root.selector) { + return false; + } + + // `selector` is either a selector string or, for menus registered on an + // element/jQuery object, the element(s) themselves - closest() takes both + return $(target).closest(root.selector).length > 0; + } + + // resolve the duration to use for one direction of the show/hide animation: + // the per-direction override (animation.showDuration / animation.hideDuration) + // when it is set, the shared animation.duration otherwise. Any value jQuery + // accepts as a duration is passed through, including 0 and "fast"/"slow". + function animationDuration(animation, key) { + if (!animation) { + return undefined; + } + + var duration = animation[key]; + return (duration === null || typeof duration === 'undefined') ? animation.duration : duration; + } + // true if target is inside one of root's sub-menus that were detached to // by op.detachSubmenus() (see #775) - used where code otherwise // relies on root.$menu[0].contains(target) to detect clicks/targets diff --git a/test/specs/animation-reopen.js b/test/specs/animation-reopen.js new file mode 100644 index 00000000..e624f764 --- /dev/null +++ b/test/specs/animation-reopen.js @@ -0,0 +1,57 @@ +const { test, expect } = require('@playwright/test'); +const { fixture } = require('../support/helpers'); + +// https://github.com/swisnl/jQuery-contextMenu/issues/739 +// The animation demo uses fadeIn/fadeOut with animation.animateOnReopen: false, +// so right clicking the second trigger while the menu is open should move the +// menu instead of fading it out and back in. +test.describe('Test animation.animateOnReopen (#739)', () => { + test('re-opening the same menu on another trigger does not replay the animation', async ({ page }) => { + await page.goto(fixture('animation.html')); + + const triggers = page.locator('.context-menu-one'); + await triggers.nth(0).click({ button: 'right' }); + await expect(page.locator('.context-menu-root')).toBeVisible(); + + // wait for the (400ms) fade in of the first open to finish + await expect + .poll(() => page.evaluate(() => window.getComputedStyle(document.querySelector('.context-menu-root')).opacity)) + .toBe('1'); + + const firstPosition = await page.locator('.context-menu-root').boundingBox(); + + // sample the menu while the second right click is handled + await page.evaluate(() => { + window.__menuSamples = []; + window.__sampler = setInterval(() => { + const menu = document.querySelector('.context-menu-root'); + if (!menu) { + window.__menuSamples.push({ display: 'removed', opacity: '0' }); + return; + } + const style = window.getComputedStyle(menu); + window.__menuSamples.push({ display: style.display, opacity: style.opacity }); + }, 5); + }); + + // the transparent modal layer covers the second trigger, so click through it + // with raw mouse coordinates, exactly like a user would + const second = await triggers.nth(1).boundingBox(); + await page.mouse.click(second.x + second.width / 2, second.y + second.height / 2, { button: 'right' }); + await page.waitForTimeout(500); + + const samples = await page.evaluate(() => { + clearInterval(window.__sampler); + return window.__menuSamples; + }); + + expect(samples.length).toBeGreaterThan(10); + const faded = samples.filter((s) => s.display === 'none' || s.display === 'removed' || Number(s.opacity) < 1); + expect(faded, 'menu should never fade out or disappear while being re-opened').toEqual([]); + + // ... and it really did move to the second trigger + await expect(page.locator('.context-menu-root')).toBeVisible(); + const secondPosition = await page.locator('.context-menu-root').boundingBox(); + expect(secondPosition.x).not.toBe(firstPosition.x); + }); +}); diff --git a/test/unit/issue-739-animation.test.js b/test/unit/issue-739-animation.test.js new file mode 100644 index 00000000..4c664b28 --- /dev/null +++ b/test/unit/issue-739-animation.test.js @@ -0,0 +1,182 @@ +// Tests for https://github.com/swisnl/jQuery-contextMenu/issues/739 +// 1. the show animation should not be replayed when the menu that is being +// opened is already visible (animation.animateOnReopen) +// 2. show and hide should be able to use separate durations +// (animation.showDuration / animation.hideDuration) + +(function () { + var showCalls = []; + var hideCalls = []; + var originalSlideDown; + var originalSlideUp; + + function fixture() { + var $fixture = $('#qunit-fixture'); + + // ensure `#qunit-fixture` exists when testing with karma runner + if ($fixture.length === 0) { + $('
').appendTo('body'); + $fixture = $('#qunit-fixture'); + } + + return $fixture; + } + + // register a menu shared by every `.trigger739` element, so re-opening it on + // another trigger re-uses the very same menu element + function createContextMenu(animation) { + var options = { + selector: '.trigger739', + items: { + copy: {name: 'Copy'}, + paste: {name: 'Paste'} + } + }; + + if (animation) { + options.animation = animation; + } + + fixture().append('
one
two
'); + $.contextMenu(options); + } + + function openOn(index) { + $('.trigger739').eq(index).contextMenu({x: 10, y: 10}); + } + + QUnit.module('issue 739 animation options', { + beforeEach: function () { + showCalls = []; + hideCalls = []; + + // spy on the default show/hide effects, so the assertions below cover + // the actual default animation methods instead of stand-ins + originalSlideDown = $.fn.slideDown; + originalSlideUp = $.fn.slideUp; + $.fn.slideDown = function (duration) { + showCalls.push(duration); + return originalSlideDown.apply(this, arguments); + }; + $.fn.slideUp = function (duration) { + hideCalls.push(duration); + return originalSlideUp.apply(this, arguments); + }; + }, + afterEach: function () { + $.fn.slideDown = originalSlideDown; + $.fn.slideUp = originalSlideUp; + + $.contextMenu('destroy'); + var $fixture = $('#qunit-fixture'); + if ($fixture.length) { + $fixture.html(''); + } + } + }); + + QUnit.test('without any animation options the default duration is used for both directions', function (assert) { + createContextMenu(); + + openOn(0); + assert.deepEqual(showCalls, [50], 'show animation ran with the default duration of 50ms'); + + $('.trigger739').eq(0).contextMenu('hide'); + assert.deepEqual(hideCalls, [50], 'hide animation ran with the default duration of 50ms'); + }); + + QUnit.test('a plain `duration` keeps applying to both show and hide', function (assert) { + createContextMenu({duration: 250}); + + openOn(0); + $('.trigger739').eq(0).contextMenu('hide'); + + assert.deepEqual(showCalls, [250], 'show animation used animation.duration'); + assert.deepEqual(hideCalls, [250], 'hide animation used animation.duration'); + }); + + QUnit.test('showDuration and hideDuration override animation.duration per direction', function (assert) { + createContextMenu({duration: 250, showDuration: 120, hideDuration: 30}); + + openOn(0); + $('.trigger739').eq(0).contextMenu('hide'); + + assert.deepEqual(showCalls, [120], 'show animation used animation.showDuration'); + assert.deepEqual(hideCalls, [30], 'hide animation used animation.hideDuration'); + }); + + QUnit.test('an unset per-direction duration falls back to animation.duration', function (assert) { + createContextMenu({duration: 250, showDuration: 0}); + + openOn(0); + $('.trigger739').eq(0).contextMenu('hide'); + + assert.deepEqual(showCalls, [0], 'showDuration: 0 is honoured instead of falling back'); + assert.deepEqual(hideCalls, [250], 'hide animation fell back to animation.duration'); + }); + + QUnit.test('by default re-opening an already visible menu replays the show animation', function (assert) { + createContextMenu(); + + openOn(0); + assert.deepEqual(showCalls, [50], 'sanity check: menu was shown with an animation'); + + // right-clicking another trigger of the same menu first hides the menu + // (from the document mousedown handler) and then shows it again, which is + // what makes the animation replay + $('.context-menu-list').trigger('contextmenu:hide'); + openOn(1); + + assert.deepEqual(showCalls, [50, 50], 'show animation was replayed with the configured duration'); + }); + + QUnit.test('animation.animateOnReopen: false does not replay the show animation for a visible menu', function (assert) { + var done = assert.async(); + createContextMenu({animateOnReopen: false}); + + // wait for the first show animation to complete, so the menu is fully + // expanded before it gets re-opened + $(document).one('contextmenu:visible', function () { + assert.deepEqual(showCalls, [50], 'sanity check: the first show is animated as usual'); + + var $menu = $('.context-menu-list'); + var height = $menu.height(); + assert.ok(height > 0, 'sanity check: the menu has a height while visible'); + + $menu.trigger('contextmenu:hide'); + openOn(1); + + assert.deepEqual(showCalls, [50, 0], 'the show animation ran with a zero duration on re-open'); + assert.ok($menu.is(':visible'), 'the menu stayed visible while being re-opened'); + + // in the middle of what would have been the hide + show animations the + // menu is still fully expanded instead of sliding + setTimeout(function () { + assert.ok($menu.is(':visible'), 'the menu is still visible after the animations would have run'); + assert.equal($menu.height(), height, 'the menu was never collapsed or re-expanded'); + done(); + }, 40); + }); + + openOn(0); + }); + + QUnit.test('animation.animateOnReopen: false still animates a menu that is not visible', function (assert) { + var done = assert.async(); + createContextMenu({animateOnReopen: false}); + + openOn(0); + + // wait until the menu is really gone (the hide animation is queued behind + // the show animation, so this takes both durations plus a little) + $(document).one('contextmenu:hidden', function () { + assert.notOk($('.context-menu-list').is(':visible'), 'sanity check: the menu is hidden again'); + + openOn(1); + assert.deepEqual(showCalls, [50, 50], 'showing a hidden menu is animated as usual'); + done(); + }); + + $('.trigger739').eq(0).contextMenu('hide'); + }); +})(); From a376fb1d42bb3c044433c062e20c6d98902bd6af Mon Sep 17 00:00:00 2001 From: bjorn Date: Wed, 29 Jul 2026 16:42:23 +0200 Subject: [PATCH 2/2] fix: only skip the re-open wait when the trigger belongs to this menu The selector-only check treated a trigger matching the same selector in another registration's context as a re-open of this menu, which would open that other menu before the current one finished hiding. Require the trigger to sit inside this registration's context as well. --- src/jquery.contextMenu.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/jquery.contextMenu.js b/src/jquery.contextMenu.js index 88961c14..eb78360e 100644 --- a/src/jquery.contextMenu.js +++ b/src/jquery.contextMenu.js @@ -2249,7 +2249,15 @@ // `selector` is either a selector string or, for menus registered on an // element/jQuery object, the element(s) themselves - closest() takes both - return $(target).closest(root.selector).length > 0; + var $trigger = $(target).closest(root.selector); + if (!$trigger.length) { + return false; + } + + // the same selector can be registered more than once, each registration + // with its own `context` and its own menu, so matching the selector alone + // isn't enough to tell that `target` is served by this very menu + return !root.context || root.context === $trigger[0] || $.contains(root.context, $trigger[0]); } // resolve the duration to use for one direction of the show/hide animation: