-
Notifications
You must be signed in to change notification settings - Fork 692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Menu clipping #749
Menu clipping #749
Conversation
1. Add zIndex option to allow overriding menu z-index for edge use cases. 2. Add logic to assign a reasonable z-index if not appending to a jQuery UI Dialog. (e.g., appending to document.body instead) 3. Go back to setting the menu position on every open in case the button has been moved (e.g. container dialog moved) from the last open. 4. Close the menu if the mouse cursor is outside the menu and the mouse wheel is turned. This fixes a scrolling issue that made the menu look detached from the button when the mouse wheel was turned. 5. Add _allowInteraction support for jQuery modal dialogs to prevent interaction issues when appending to document body to prevent menu truncation. Note these changes do not explicitly fix the menu truncation issue. However, they do make it much easier to address menu truncation by just explicitly setting the appendTo option to document.body.
It looks like these changes are bringing the z-index calculation in, but not changing the append behavior yet? |
@mlh758 This is a compromise that lets you "have it both ways". Essentially, I am not explicitly changing the append behavior as it is fine for the situation where you append to a "tall" dialog where clipping would not occur. I have looked at the jQuery UI code, so I have an idea of what they are doing w/ .ui-front Re: stacking order. However, while the original code did allow the appendTo element to be explicitly specified for some use cases (e.g. append to document body for short dialogs), it really did not provide sufficient support for setting the z-index properly for that use case and it did not address modal dialog interaction for that use case. In essence, this PR does not really change anything for the appendTo ".ui-front use case", as I think that is probably fine for many situations if you also set the position within property for the inner scrolling div of the dialog. What this PR addresses is the appendTo's that are not '.ui-front'... a reasonable z-index is set for those or you can override it. Also, the _allowInteraction extension is set for jQuery UI dialogs to prevent interaction problems. I came to the conclusion in my thinking that there really was no good reliable way to have the code automatically determine whether to appendTo .ui-front or document body, so we should leave that decision to a developer. We should provide some guidance in a wiki to assist that decision. |
@mlh758 Do you have any specific requests for changes? |
No, I've just been really busy the last couple of days and haven't had a chance to finish testing it. |
Looks good, merged it. I updated the wiki for the |
Resolves #739
Note these changes do not directly fix the menu truncation issue. However, they do make it much easier to address menu truncation by just explicitly setting the appendTo option to document.body, since the z-index would automatically get set to a reasonable value in that case. The _allowInteraction support is needed for this use case since typically jQuery UI modal dialogs only allow interactions with elements that are contained in the dialog. (When appending the menu to document.body, typically this would be a case where normal interaction with the menu would be expected to be blocked by the jQuery UI modal dialog code.)