docs: make the inline SVG icon example idempotent

[bbrala]

Addresses the Codex review point on #794 that was never picked up before that PR merged: #794 (review)

Problem

The example added in #794 prepends an <svg> unconditionally:

icon: function (opt, $itemElement) {
    $itemElement.prepend('<svg class="context-menu-icon" ...>...</svg>');
    return 'context-menu-icon-inline';
}

A callback icon is invoked twice before the menu is ever visible and once more on every open after that:

Where	What
src/jquery.contextMenu.js:1955	op.create() calls the callback, then prepends
src/jquery.contextMenu.js:1493	op.show() calls op.update() on every show
src/jquery.contextMenu.js:2195	op.update() calls the callback again

So the item carries two icons on its very first display, and gains another with every subsequent open. The returned context-menu-icon-inline class reads as a done-marker, but nothing ever checked it.

Fix

Guard on the injected element, which is what makes the callback safe to re-run:

if (!$itemElement.children('svg.my-inline-icon').length) {
    $itemElement.prepend('<svg class="my-inline-icon" ...>...</svg>');
}

Two related doc fixes came out of it:

• The example put context-menu-icon on the child <svg>. That is the plugin's own item-level class (classNames.icon), which carries an absolutely positioned ::before via the base-context-menu-icon mixin. Changed to a class of the reader's own.
• items.md never said the callback re-runs, which is what allowed the example to be written this way in the first place. It says so now and points at the guard pattern. The canonical example there needs no guard because $itemElement.html() replaces content rather than adding to it.

Why docs-only, and no plugin change

The re-invocation is deliberate and documented behaviour: it is what lets a callback icon reflect current state, the same way function-based disabled/visible/name do. The contract is "your callback may run repeatedly", and the canonical items.md example already honours it via .html(). #794 simply broke that contract.

Making the plugin de-duplicate prepended content would be a behaviour change affecting an undocumented return shape (returning an element rather than a class string), for no benefit over fixing the example. This is a bad example, not a bad plugin.

Verification

npm run docs:build is clean, and the new cross-reference resolves: items.html emits href="customize#using-your-own-svg-icons-without-a-build-step" and customize.html carries id="using-your-own-svg-icons-without-a-build-step".

No source or test changes, so nothing in the matrix is affected.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 664c295161

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Qualify dynamic icon-class state handling

When an icon callback returns a different class string as state changes, the icon does not reliably reflect only the current state: op.update() removes item._icon, which is the creation-time result, but never assigns the new iconResult back to item._icon, so classes returned by successive opens accumulate on the item. This new guidance therefore encourages a broken dynamic-class pattern unless the callback reuses one class or manages stale classes itself; document that limitation or update the implementation to replace the previous result.

Useful? React with 👍 / 👎.

[bbrala]

Thanks, this one was right and it landed on my own change rather than just the pre-existing code, so I've fixed it rather than documented it.

The guidance I added said a callback icon re-runs "so that the icon can reflect current state". As you point out, a callback whose class tracks that state did not actually work, so that sentence was selling a broken pattern.

Reproduced it first (test/unit/issue-794-icon-callback-classes.test.js):

create  -> _icon='state-one', item has state-one
update  -> removeClass('state-one'), addClass('state-two')   _icon unchanged
update  -> removeClass('state-one') no-op, addClass('state-three')
           item now carries state-two AND state-three

The test failed with ["state-two", "state-three"] where one class was expected.

Fixed rather than documented. Writing down "manage stale classes yourself" would enshrine the wart, and the alternative was to soften the docs and leave a callback icon unable to express changing state at all, which is the entire reason it gets re-evaluated. op.update() now assigns the result back to item._icon, so the next update removes the class the previous one applied.

On backwards compatibility, since that is the bar here:

• A callback returning a constant string, the common case by far, is unaffected: removing and re-adding the same class is a no-op either way. Pinned by a second test that also asserts the class is not duplicated.
• Only a callback returning different strings changes behaviour, and only by dropping stale classes, which is the bug itself.
• A callback returning an element rather than a string is unaffected. removeClass() ignores a non-string argument, exactly as before.
• item._icon is internal, read only in op.create() and op.update().

Unit 139/139 and acceptance 38/38 green locally, and the docs now state that a returned class is swapped on each call while anything the callback does to $itemElement is not, so only that part needs a guard.

Two things found while tracing this, both left out as out of scope for this PR:

1. A callback returning an element still accumulates. op.update() calls $item.prepend(item._icon) on every update and removeClass() cannot undo it. Undocumented return shape, so it needs its own issue and a deliberate decision.
2. $.fn.contextMenu('update') is a no-op. There is no 'update' branch in $.fn.contextMenu, so $(sel).contextMenu('update') falls through to else if (operation) and just runs removeClass('context-menu-disabled'). The 2.6.1 changelog documents $('.context-menu-one').contextMenu('update') as the single-menu form, so that documented call has silently done nothing. Only the global $.contextMenu('update') works, which is what the tests here use.