Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

dropdownAttached option. #265

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

It would be great to get this brought into master, as I have been having trouble with the way the drop down is appended to the body and positioned alongside the link control.


Added dropdownAttached option (false by default) which can simplify integration by avoiding issues where the drop down can break away from the link control when used inside jQuery UI dialogs etc.

Note that this setting does NOT allow for the newly introduced feature where the drop down is positioned above if there is no space, since it essentially prevents positionDropDown from being called altogether.

Added dropdownAttached option
Added dropdownAttached option (false by default) which can simplify integration by avoiding issues where the drop down can break away from the link control when used inside jQuery UI dialogs etc.

Note that this setting does NOT allow for the newly introduced feature where the drop down is positioned above if there is no space, since it essentially prevents positionDropDown from being called altogether.
Contributor

ivaynberg commented Jul 25, 2012

shouldnt we instead concentrate on fixing the actual issues you have instead of introducing, what basically is, a workaround?

that's a fair point, I'm just not sure how easy that would be, since it's related to the fact that jQuery UI is using some kind of fixed positioning to keep the dialog content in place as the document scrolls. Since the drop down is also attached at the same level, it scrolls away from the link element in the dialog. It makes the whole thing feel a bit less robust...

I guess I favoured the "option" approach as it keeps it simple in my integration. The drop down above thing is also not particularly appealing in my integration, so this kind of killed two birds with one stone :P

All good if you'd prefer not to go this way, I'm happy to keep it forked.

Contributor

ivaynberg commented Jul 25, 2012

lets keep it forked for now. can you please create a jsfiddle though so i can see the scrolling problem and if there is a relatively sane way around it?

Contributor

ivaynberg commented Jul 25, 2012

related to #149

looking through the issues, the body append and positioning was introduced as a response to #37.

I feel the append to body causes more problems than it's worth, and IE7 is barely worth supporting. Especially since select2 is a widget that uses JavaScript heavily, so I imagine its not the speediest thing in IE7 anyway.

I'd also note that Harvest didn't bother supporting it with Chosen :)

Contributor

ivaynberg commented Jul 25, 2012

if you follow the issue comments you will see that this started out as a problem in IE7 but was also later discovered to have problems in chrome and others.

without this select2 also cannot be used in a modal because the dropdown part will never overlay the modal since it will be inside, so in narrow modals select2 cannot be used.

opening up is also an important and often request feature, although i believe we would be able to hack that with the dropdown inline, but not as easily.

I'd also note that Harvest didn't bother supporting it with Chosen :)

they didnt bother supporting ajax or templating either, thats why we are here :)

like i said, i think we should find a fix with the dropdown model like it is now. btw, why are users scrolling the page when the dropdown is opened?

Owner

kevin-brown commented Jul 25, 2012

Can't we solve this whole scrolling issue by closing the drop-down when the user scrolls?

It is what browsers do already, and it could be an option to not have the dropdown do this.

Contributor

ivaynberg commented Jul 25, 2012

if we could detect the scrolling then we could also reposition the dropdown. i havent looked into it yet, it may be really easy....

Contributor

ivaynberg commented Aug 8, 2012

i think we can close this with the fix to #149

@ivaynberg ivaynberg closed this Aug 8, 2012

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