fix: keep Font Awesome icons centered in a menu with a title

.github/workflows/tests.yml

@@ -74,6 +74,13 @@ jobs:
         if: steps.playwright-cache.outputs.cache-hit == 'true'
         run: npx playwright install-deps chromium
 
+      # The fixture pages load the plugin from dist/, which is only rebuilt at
+      # release time and is therefore usually behind src/ on a pull request.
+      # Build it here so the acceptance tests exercise the current source
+      # instead of the last released bundle.
+      - name: Build dist
+        run: npm run build
+
       - name: Generate versioned test fixtures
         run: npm run test:fixtures
 

documentation/_data/nav.js

@@ -49,6 +49,7 @@ module.exports = [
       { id: 'input', text: 'Input Commands', url: '/demo/input.html' },
       { id: 'keeping-contextmenu-open', text: 'Keeping the context menu open', url: '/demo/keeping-contextmenu-open.html' },
       { id: 'menu-title', text: 'Menus with titles', url: '/demo/menu-title.html' },
+      { id: 'menu-title-fontawesome', text: 'Menu title with Font Awesome icons', url: '/demo/menu-title-fontawesome.html' },
       { id: 'menu-promise', text: 'Menu with promise', url: '/demo/menu-promise.html' },
       { id: 'on-dom-element', text: 'Context Menu on DOM Element', url: '/demo/on-dom-element.html' },
       { id: 'sub-menus', text: 'Submenus', url: '/demo/sub-menus.html' },

documentation/demo.md

@@ -73,6 +73,7 @@ title: jQuery contextMenu — Demo gallery
 *   [Input Commands](demo/input.html)
 *   [Custom Command Types](demo/custom-command.html)
 *   [Menus with titles](demo/menu-title.html)
+*   [Menu title with Font Awesome icons](demo/menu-title-fontawesome.html)
 *   [Importing HTML5 <menu type="context">](demo/html5-import.html)
 *   [HTML5 Polyfill](demo/html5-polyfill.html)
 *   [HTML5 Polyfill (Firefox)](demo/html5-polyfill-firefox8.html)

documentation/demo/menu-title-fontawesome.md

@@ -0,0 +1,95 @@
+---
+currentMenu: menu-title-fontawesome
+---
+
+# Demo: Menu Title with Font Awesome icons
+
+
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+
+
+- [Example CSS](#example-css)
+- [Example code](#example-code)
+- [Example HTML](#example-html)
+
+<!-- END doctoc generated TOC please keep comment here to allow auto update -->
+
+<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.15.4/css/all.min.css">
+
+Combining a [menu title](menu-title.html) with [Font Awesome icons](fontawesome-icons.html).
+The icons stay vertically centered in their item, whether the menu has a title or not.
+
+<span class="context-menu-fa-title btn btn-neutral">right click me (with title)</span>
+<span class="context-menu-fa-plain btn btn-neutral">right click me (no title)</span>
+
+## Example CSS
+
+Note the child combinator in `> :first-child`: the top margin is only meant for
+the first menu item, so that the title has room. Without it the rule also
+matches the `<i>` element that the plugin creates for a Font Awesome icon,
+and the label of an input item, since those are the first child of their own
+menu item.
+
+<style type="text/css" class="showcase">
+    /* menu header */
+    .menu-title-fa:before {
+        content: "Font Awesome title";
+        display: block;
+        position: absolute;
+        top: 0;
+        right: 0;
+        left: 0;
+        background: #DDD;
+        padding: 2px;
+
+        font-family: Verdana, Arial, Helvetica, sans-serif;
+        font-size: 11px;
+        font-weight: bold;
+    }
+    .menu-title-fa > :first-child {
+        margin-top: 20px;
+    }
+</style>
+
+## Example code
+
+<script type="text/javascript" class="showcase">
+$(function(){
+    var items = {
+        "edit": {name: "Edit", icon: "fas fa-edit"},
+        "cut": {name: "Beer", icon: "fas fa-beer"},
+        "copy": {name: "Cloud download", icon: "fas fa-cloud-download-alt"},
+        "paste": {name: "Certificate", icon: "fas fa-certificate"}
+    };
+
+    // menu with a title provided by CSS
+    $.contextMenu({
+        selector: '.context-menu-fa-title',
+        className: 'menu-title-fa',
+        callback: function(key, options) {
+            var m = "clicked: " + key;
+            window.console && console.log(m) || alert(m);
+        },
+        items: items
+    });
+
+    // the same menu without a title, for comparison
+    $.contextMenu({
+        selector: '.context-menu-fa-plain',
+        callback: function(key, options) {
+            var m = "clicked: " + key;
+            window.console && console.log(m) || alert(m);
+        },
+        items: items
+    });
+});
+</script>
+
+## Example HTML
+
+```html
+<span class="context-menu-fa-title btn btn-neutral">right click me (with title)</span>
+
+<span class="context-menu-fa-plain btn btn-neutral">right click me (no title)</span>
+```

documentation/demo/menu-title.md

@@ -32,6 +32,12 @@ currentMenu: menu-title
 
 ## Example CSS
 
+The title is rendered with a `:before` pseudo element on the menu itself, and
+the first menu item is given a top margin so the title has room. Keep the child
+combinator in `> :first-child`: a plain descendant selector would also match the
+first child *inside* a menu item, such as the `<i>` element created for a Font
+Awesome icon or the `<label>` of an input item, and push those down as well.
+
 <style type="text/css" class="showcase">
     /* menu header */
     .css-title:before {
@@ -48,7 +54,7 @@ currentMenu: menu-title
         font-size: 11px;
         font-weight: bold;
     }
-    .css-title :first-child {
+    .css-title > :first-child {
         margin-top: 20px;
     }
 
@@ -67,7 +73,7 @@ currentMenu: menu-title
         font-size: 11px;
         font-weight: bold;
     }
-    .data-title :first-child {
+    .data-title > :first-child {
         margin-top: 20px;
     }
 </style>

src/sass/jquery.contextMenu.scss

@@ -52,11 +52,28 @@
     font-family: inherit;
     line-height: inherit;
 
+    // The <i>/<svg> element is created and positioned by this plugin, so its
+    // box is pinned down completely instead of relying on whatever the icon
+    // library (or the surrounding page) happens to leave it at.
+    //
+    // `height`/`line-height` make the box exactly one em tall so the negative
+    // top margin can center it against `top: 50%`, which keeps the icon
+    // centered for any item padding or font size. Centering via `margin`
+    // rather than `transform` leaves Font Awesome's own transform helpers
+    // (fa-rotate-*, fa-flip-*) working.
+    //
+    // Resetting `margin` also protects the icon from page level rules such as
+    // the "menu title" recipe (see the menu-title demo), which puts a top
+    // margin on the menu's first child to make room for the title and used to
+    // match this element too, pushing every icon down (#738).
     i, svg {
       color: $context-menu-icon-color;
+      height: 1em;
       left: 0.5em;
+      line-height: 1;
+      margin: -0.5em 0 0;
       position: absolute;
-      top: 0.3em;
+      top: 50%;
     }
 
     &.context-menu-hover {

test/specs/menu-title-icon-alignment.js

@@ -0,0 +1,126 @@
+const { test, expect } = require('@playwright/test');
+const { fixture } = require('../support/helpers');
+
+// Regression test for https://github.com/swisnl/jQuery-contextMenu/issues/738
+//
+// The documented way to give a menu a title is a `:before` pseudo element on
+// the menu plus a top margin on its first child, to make room for that title.
+// A Font Awesome icon is rendered as an <i> element inside the menu item, so
+// it is the first child of its own <li>. A descendant `:first-child` selector
+// therefore matched every icon as well and pushed them all down by the title's
+// height, leaving the icons hanging below their label.
+//
+// The plugin now pins the icon element's own box (including its margin), so
+// the icon stays vertically centered in its item regardless of what the page
+// does to first children.
+
+// The icon and its item may legitimately differ by a sub-pixel amount because
+// of fractional em based paddings.
+const TOLERANCE = 1.5;
+
+// Reads the vertical center of every Font Awesome icon and of the menu item it
+// belongs to, from the menu that is currently visible.
+function readIconOffsets(page) {
+  return page.evaluate(() => {
+    const menu = Array.from(document.querySelectorAll('.context-menu-list'))
+      .find((el) => window.getComputedStyle(el).display !== 'none');
+
+    return Array.from(menu.children)
+      .filter((item) => item.querySelector('i'))
+      .map((item) => {
+        const itemBox = item.getBoundingClientRect();
+        const iconBox = item.querySelector('i').getBoundingClientRect();
+
+        return {
+          label: item.textContent.trim(),
+          offset: (iconBox.top + iconBox.height / 2) - (itemBox.top + itemBox.height / 2),
+        };
+      });
+  });
+}
+
+// The built-in icon font is drawn in a `::before` pseudo element, which has no
+// box to measure directly. It is centered with `top: 50%` plus a translate of
+// half its own height, so its center sits exactly at the used value of `top`,
+// which must be half the item's height.
+function readBuiltInIconOffsets(page) {
+  return page.evaluate(() => {
+    const menu = Array.from(document.querySelectorAll('.context-menu-list'))
+      .find((el) => window.getComputedStyle(el).display !== 'none');
+
+    return Array.from(menu.children)
+      .filter((item) => item.classList.contains('context-menu-icon'))
+      .map((item) => ({
+        label: item.textContent.trim(),
+        offset: parseFloat(window.getComputedStyle(item, '::before').top) -
+          item.getBoundingClientRect().height / 2,
+      }));
+  });
+}
+
+function expectCentered(offsets) {
+  expect(offsets.length).toBeGreaterThan(0);
+  for (const { label, offset } of offsets) {
+    expect(Math.abs(offset), 'icon of item "' + label + '" is off center by ' + offset + 'px')
+      .toBeLessThanOrEqual(TOLERANCE);
+  }
+}
+
+test.describe('Test icon alignment in a menu with a title (#738)', () => {
+  test('Font Awesome icons are centered in a menu with a title', async ({ page }) => {
+    await page.goto(fixture('menu-title-fontawesome.html'));
+    await page.click('.context-menu-fa-title', { button: 'right' });
+
+    const menu = page.locator('.context-menu-list.menu-title-fa');
+    await expect(menu).toBeVisible();
+
+    expectCentered(await readIconOffsets(page));
+  });
+
+  test('Font Awesome icons are centered in a menu without a title', async ({ page }) => {
+    await page.goto(fixture('menu-title-fontawesome.html'));
+    await page.click('.context-menu-fa-plain', { button: 'right' });
+
+    await expect(page.locator('.context-menu-list:visible')).toBeVisible();
+
+    expectCentered(await readIconOffsets(page));
+  });
+
+  test('Font Awesome icons survive a descendant :first-child title rule', async ({ page }) => {
+    await page.goto(fixture('menu-title-fontawesome.html'));
+
+    // The title recipe as it was documented before #738: a descendant
+    // selector rather than a child combinator. Plenty of pages in the wild
+    // still use it, so the plugin's own icon styling has to win.
+    await page.addStyleTag({ content: '.menu-title-fa :first-child { margin-top: 20px; }' });
+
+    await page.click('.context-menu-fa-title', { button: 'right' });
+
+    const menu = page.locator('.context-menu-list.menu-title-fa');
+    await expect(menu).toBeVisible();
+    // sanity check: the rule under test really is applied to the menu's first
+    // item, so the title still has its room
+    await expect(menu.locator('li').first()).toHaveCSS('margin-top', '20px');
+
+    expectCentered(await readIconOffsets(page));
+  });
+
+  test('built-in icon font is centered in a menu with a title', async ({ page }) => {
+    await page.goto(fixture('menu-title.html'));
+    await page.click('.context-menu-two', { button: 'right' });
+
+    const menu = page.locator('.context-menu-list.css-title');
+    await expect(menu).toBeVisible();
+
+    expectCentered(await readBuiltInIconOffsets(page));
+  });
+
+  test('built-in icon font is centered in a menu without a title', async ({ page }) => {
+    await page.goto(fixture('menu-title.html'));
+    await page.click('.context-menu-one', { button: 'right' });
+
+    await expect(page.locator('.context-menu-list:visible')).toBeVisible();
+
+    expectCentered(await readBuiltInIconOffsets(page));
+  });
+});