feat: make the trigger reachable from item-level events handlers - #802
Merged
Conversation
Item `events` handlers are bound with their menu's options object as jQuery event data, so `e.data.$trigger` already exposed the trigger for items in the root menu. Items in a sub-menu got the sub-menu's own options object, which never had `$trigger` set, leaving no supported way to reach the element that opened the menu. op.show() now stamps the trigger onto the root options object and every already-created sub-menu's options object, and op.create() inherits it from the root for sub-menus built later (a resolving promise, for instance). The `events` documentation now spells out what `this` and `e.data` contain, and its example was fixed: it nested the item under `events` instead of `items`. Added a demo showing a text input renaming the button that opened the menu. Closes #729
Member
Author
|
👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Reported in #729: a menu with a text input wants to write the typed value back onto the button that opened the menu, but there was no way to get at the trigger from inside the item's
eventshandler.What was already possible
Item
eventshandlers are bound with$input.on(item.events, opt), so the menu's options object arrives as jQuery event data. For an item in the root menu that object is the root options object, andop.show()sets$triggeron it, soe.data.$triggeralready worked. It was documented initems.md, but the example there was broken (it nested the item undereventsinstead ofitems), so it was easy to miss.For an item in a sub-menu the event data is the sub-menu's own options object, and nothing ever set
$triggeron it.e.data.$triggerwasundefinedthere.Change
op.setTrigger()stamps the trigger onto the root options object and, recursively, onto the options object of every sub-menu that has already been created.op.show()calls it in place of the old single assignment.op.create()inherits$triggerfrom the root when it creates a sub-menu, so sub-menus built after the menu was shown (a resolving promise, for instance) get it too.No handler signature or argument meaning changes:
e.datais still the menu's own options object, it just now always carries$trigger.Docs
documentation/docs/items.md: theeventssection now states whatthisande.dataare, notes thate.data.$triggerfollows the currently visible menu's trigger even when many elements share one menu definition, fixes the broken example, and adds a write-back example.documentation/demo/input-rename-trigger.mdwith the reporter's exact scenario: two buttons sharing one left-click menu, a text input whosefocusoutrenames the button that was clicked.Tests
test/unit/issue-729-trigger-in-item-events.test.js:e.data.$triggerresolves to the correct element when two triggers share one menumaster)master)focusoutwrites the input value onto the trigger and leaves the other trigger alonenpm test(46 assertions) and the Playwright suite (22) are green,npm run docs:buildsucceeds.Closes #729