feat(dropdown): Add dropup behavior #3703
Conversation
Makes dropdown menu go upwards if it's at the bottom of the browser window. Moves DOM logic to directive. CLoses angular-ui#1317
|
This is some nice work! I was thinking wouldn't is be best to add this sort of functionality to the $position service in order to allow tooltip and popover to have the same sort of logic? Also, we need some tests for this beauty. |
|
@rvanbaalen Thanks! Moving these calculations to $position service is a great idea. I'd like work on it after creating tests for this one. Would it work? |
|
@rvanbaalen I've added tests for this pr. |
| dropdownMenuHeight = $position.position(dropdownMenu).height, | ||
| dropdownMenuBottom = dropdownOffset + dropdownHeight + dropdownMenuHeight; | ||
|
|
||
| if ( appendToBody !== undefined ) { |
There was a problem hiding this comment.
Any reason this isn't just if (appendToBody)?
There was a problem hiding this comment.
@wesleycho Here if you check for falseness dropdown.attr('dropdown-append-to-body') evaluates to false even if dropdown-append-to-body attribute is applied to dropdown element.
So we need to specifically check for its undefined state.
| var dropdown, appendToBody, | ||
| dropdownMenu = element; | ||
|
|
||
| scope.$watch(dropdownCtrl.isOpen, function( isOpen ) { |
There was a problem hiding this comment.
Any reason why this $watch isn't in the controller?
There was a problem hiding this comment.
@wesleycho I believe Link function is more appropriate, because most of the logic here is DOM-related (and Link function and Controller share the same scope in this case).
There was a problem hiding this comment.
I think I would favor this being in the controller - this adds an additional $watch for what seems to be unnecessary reasons.
- Due to switch to raw `addEventListener`, changed to add and remove listeners to event triggers by iteration Fixes angular-ui#4371 Closes angular-ui#4384
- Change to pass object due to how `&` binding works Fixes angular-ui#4386 Closes angular-ui#4387
- Added ability to add a class to the most recently opened modal window. Note that even if different classes are specified, the class will only be present if the modal is the most recently opened modal, i.e. if modal1 was opened with a top class of `foo`, and modal2 is opened afterwards with a top class of `bar`, modal2 will have the class `bar` for the modal window, and modal1 will not have the class `foo`. Closes angular-ui#2524
- Hide tooltip when `esc` is hit for accessibility Closes angular-ui#4367 Resolves angular-ui#4248
This reverts commit bf63cef.
- Check the correct DOM node for animation settings Closes angular-ui#4325
- Removes unnecessary $digest inside $timeout Closes angular-ui#4521
- Adds uib- prefix to `dropdownConfig` - Restores functionality to deprecated directives Closes angular-ui#4514
|
Can you rebase this off of current |
- Expose `AccordionController` with deprecation message Closes angular-ui#4524
- Re-expose `AlertController` with deprecation message Closes angular-ui#4525
Makes dropdown menu go upwards if it's at the bottom of the browser window. Moves DOM logic to directive. CLoses angular-ui#1317
|
Is there any possible way you can abstract this logic for determining whether to do the up or down to the |
|
@wesleycho Rebased it. |
|
Looks like there are conflicts still - also looks like the history got busted - maybe try filing a new PR with the history fixed? You might have rebased when there was a merge commit present - the merge commit causes problems with history, which is why we always rebase. |
|
@wesleycho Here is a new PR: #4534 |
|
Closing as this is superceded by the referenced PR |
Makes dropdown menu go upwards if it's at the bottom of the browser window and is being partially hidden.
Moves DOM logic to directive.
Closes #1317