make the postion of popup dynamic #578

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

clarkbox commented Jan 19, 2012

When using popup(), with the trigger option set, the popup is always positioned relative to the first matching trigger element. With this modification, the popup will be positioned relative to the event target, making it possible to have a single popup with multiple targets.

Owner

scottgonzalez commented Jan 19, 2012

At a quick glance this seems fine. Someone with more knowledge of popup, such as @jzaefferer should review this. We should also verify that there are no a11y concerns with many triggers owning the same popup.

Owner

jzaefferer commented Jan 19, 2012

It should be possible to open a popup programmatically through that open method, similar to tooltip. With the change in place, you'd have to pass a valid event target.

What's the usecase for a single popup with multiple targets? How would you set that up, considering that you need to call popup on the popup element, not the trigger?

Last but not least, patches for popup should go into the menubar branch, where popup now lives.

Member

clarkbox commented Jan 19, 2012

the use case i am faced with is using popup() in conjunction with menu() to create a context menu within a grid.

imagine a "settings" button in each row of a grid. ideally, that settings button would open the same popup/menu element, and the select method would determine (via this.closest('tr')...) the row that was clicked.

say the button has a class of '.settingBtn'... and the popup element is '.pop'. keeping in mind there is only one .pop element, and many '.settingBtn' elements...

$( '.pop' ).popup( {trigger: $('.settingBtn')} ); (EDIT)

I will create a test and demo, and move this over to the menubar branch.

Owner

scottgonzalez commented Jan 19, 2012

It's probably fine to default to this.options.trigger.eq( 0 ) for programmatic opens. This could be similar to tooltip, where it supports one or many, and you get different functionality available based on your use case.

Owner

jzaefferer commented Jan 19, 2012

Makes a lot more sense now. A table demo would be quite useful, too.

Member

clarkbox commented Jan 20, 2012

ok this is not as straight forward as it seemed (SURPRISE!).

just modifying the first few lines of the open method (to the following)...:

    var target = event ? event.target : this.options.trigger;        
    var position = $.extend( {}, {
        of: target
    }, this.options.position );

...is only a half ass solution. after opening a popup via one trigger, than immediately clicking on another (different) trigger element, the popup will close, but then will not open on the second trigger. This is due to the return within the mousedown event on line 114. the return will have to be done more smartly, and only return IF clicking on the same trigger twice.

I am going to continue modifying popup() accordingly. just wanted to log the status, and open this up to other suggestions.

Owner

jzaefferer commented Jan 21, 2012

@clarkbox regarding your irc ping, I'm not seeing a new commit here.

Lemme know when you got something to look it. And yeah, clicking that other trigger should open the popup.

Member

clarkbox commented Jan 24, 2012

closing this request. replaced by #582

@clarkbox clarkbox closed this Jan 24, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment