-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[Dropdown] Add back closeOnClick functionality for dropdowns. #7353
[Dropdown] Add back closeOnClick functionality for dropdowns. #7353
Conversation
Use this functionality heavily on our site. Unsure why it was excluded in 6 so I disabled by default.
Looks like this breaks anything with data-close in dropdown. Going to fix and repush. |
Also include edge case for when event target was removed earlier in the event chain.
Updated. Solution is a little heavy handed, but powerful. Suggestions? |
seems like it would be much faster and less code to say: if(_this.$element.find(e.target).length){
return;
}
_this.close();
$body.off... for the check if the click was in the dd or not. Otherwise, I think this is a fine feature. |
@zurbchris Updated |
Found another bug with my addBodyHandler, and a unused settings var in dropdown.scss |
This is a pretty good feature. I've added it to the 6.1 Milestone list, and there will be a new 6.1 specific branch shortly if you could change this pr to that branch. Don't worry about compiling a new copy of the js, we'll take care of it. |
@zurbchris #7478 |
I'll take it in case the new dropdown isn't complete in time. I do like the feature :) |
[Dropdown] Add back closeOnClick functionality for dropdowns.
@tjhiggins Thanks! |
Use this functionality heavily on our site.
Unsure why it was excluded in 6 so I disabled by default.
Shamelessly copied this commented out code from DropdownMenu. Then retooled to fit the new flow.
Do I also have to rebuild foundation.js in this pr?