Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Template url dropdown doesn't work once apply dropdown-append-to-body. #4240

Closed
neesonqk opened this issue Aug 20, 2015 · 13 comments
Closed

Comments

@neesonqk
Copy link

Here is the Plunker

As you can see, once remove dropdown-append-to-body, this dropdown menu starts to work.

@neesonqk neesonqk changed the title Dropdown doesn't work once apply dropdown-append-to-body. Template url dropdown doesn't work once apply dropdown-append-to-body. Aug 20, 2015
@wesleycho wesleycho modified the milestones: 0.13.x, 0.13.4 (Performance) Aug 20, 2015
@neesonqk
Copy link
Author

I've also experienced that once put a dropdown button with dropdown-append-to-body into a scrollable container, the dropdown menu won't follow this button anymore while scrolling. I understood that must be caused by appending this dropdown menu to body, in the document it says ...This is useful when the dropdown button is inside a div with overflow: hidden, and the menu would otherwise be hidden..., so I'm wondering if this is a bug or it won't support let's say overlow-y: scroll? thanks.

See this Plunker

@wesleycho
Copy link
Contributor

Posting my findings atm in case it's of use - here is my Plunker.

The culprit seems to be the replaceWith methods in lines 359 and 373 in the ui-bootstrap.js script in the Plunker.

@wesleycho
Copy link
Contributor

I figured it out - what is happening is that the body tag needs the dropdown class, and open needs to toggle on body when dropdownAppendToBody is used. This should be a simple fix.

@spiffy2man
Copy link

This has not been fixed. This "fix" is adding the dropdown class to the body. In some instances this is messing up my pages, because the bootstrap dropdown class is position relative.

I have several dropdowns in a table, one per row. Now my table disappears unless I remove the dropdown class manually. Also, the mutliple dropdowns all seem to pull in the same data now. This was working correctly with version 0.13.2. My code is now broken in version 0.13.4.

By removing the "dropdown-append-to-body" directive, my code works correctly again, except that the dropdowns are all contained within each table row, which is just unappealing, and doesn't provide for a good user experience.

@wesleycho
Copy link
Contributor

You should override the CSS on the app level - it was marked as a breaking change for a reason.

@spiffy2man
Copy link

So, it was working just fine before and now it isn't. Well, that is a breaking change. What did you fix? Can we try to fix it again?

The provided plunker works fine if the ul is an actual ul with contents instead of having the template-url="dropdown.html" attribute and value.

http://plnkr.co/edit/hzmCNk8hjiU87KOSjZEi?p=preview

@wesleycho
Copy link
Contributor

template-url is a documented feature: https://github.com/angular-ui/bootstrap/blob/master/src/dropdown/docs/readme.md .

It is clearly a bug.

The only real question here is whether it should be solved by a user's CSS or by the library. I made a call to do so in the library itself. This is easy to fix from the user's side if one wants to override this CSS by just creating styles under the body.dropdown selector or something similar.

@spiffy2man
Copy link

How about not sacrificing anything and calling the css modifications after getting the templateUrl. Place the code inside this if statement ( if (appendToBody && self.dropdownMenu) { ) into a method, called modifyCss (or whatever). Call the method as needed, like inside of the $compile method after getting the template.

I have not fully tested the code below, but it does provide the intended functionality without any breaking changes that I can find.

http://plnkr.co/edit/l6xyeH71SR8hGgCzwGTE?p=preview

   var modCss = function() {
          if (appendToBody && self.dropdownMenu) {
              var pos = $position.positionElements(self.$element, self.dropdownMenu, 'bottom-left', true);
              var css = {
                      top: pos.top + 'px',
                      display: isOpen ? 'block' : 'none'
              };

              var rightalign = self.dropdownMenu.hasClass('dropdown-menu-right');
              if (!rightalign) {
                  css.left = pos.left + 'px';
                  css.right = 'auto';
              } else {
                  css.left = 'auto';
                  css.right = (window.innerWidth - (pos.left + self.$element.prop('offsetWidth'))) + 'px';
              }

              self.dropdownMenu.css(css);
          }
      };
      if (isOpen) {
          if (self.dropdownMenuTemplateUrl) {
              $templateRequest(self.dropdownMenuTemplateUrl).then(function(tplContent) {
                  templateScope = scope.$new();
                  $compile(tplContent.trim())(templateScope, function(dropdownElement) {
                      var newEl = dropdownElement;
                      self.dropdownMenu.replaceWith(newEl);
                      self.dropdownMenu = newEl;
                      modCss();
                  });
              });
          } else {
              modCss();
          }
          scope.focusToggleElement();
          dropdownService.open(scope);
      } else {
          if (self.dropdownMenuTemplateUrl) {
              if (templateScope) {
                  templateScope.$destroy();
              }
              var newEl = angular.element('<ul class="dropdown-menu"></ul>');
              self.dropdownMenu.replaceWith(newEl);
              self.dropdownMenu = newEl;
          }

          dropdownService.close(scope);
          self.selectedOption = null;
          modCss();
      }

@wesleycho
Copy link
Contributor

First, dial back the attitude - taking a hostile first tone is an almost sure way to reduce the likelihood of engagement, much less any positive resolution, especially in open source, where there is no intention of making decisions on vindictiveness. I'm generally here to try to help, but this type of response discourages doing so.

I'm not against trying to find another solution - the discussion just needs to be framed in a productive way.

@jaydiablo
Copy link
Contributor

I don't know what the mechanism is exactly, but with 0.13.4 and dropdown-append-to-body, when I open a dropdown the bootstrap date range picker I have on the same page opens as well (this is the picker: https://github.com/dangrossman/bootstrap-daterangepicker this is the angular wrapper for it: https://github.com/diablomedia/ng-bs-daterangepicker).

If I edit the "openClass" constant in "dropdownConfig" module to something like "dropdown-open", this behavior stops. So it seems to be directly related to adding/removing the "open" class to the body element. But I don't see anywhere in the picker or in the wrapper where it would be listening for such a change, so perhaps there's something deeper.

0.13.3 and dropdown-append-to-body don't exhibit this behavior.

Is it possible to use a different class name on the body element? Are you using "open" and "dropdown" because they're classes defined in boostrap.css?

On that note, is it possible to modify that constant without editing the angular-bootstrap code directly? My dropdowns work regardless of what this classname is (I haven't looked into the plunkr that first exhibited the bug), but I can't upgrade to 0.13.4 and use append-to-body dropdowns unless I can change this classname through a provider or other mechanism.

Thanks!

@wesleycho
Copy link
Contributor

That sounds reasonable - can you open a separate issue with that?

@jaydiablo
Copy link
Contributor

Ah, the reason the daterange picker shows is because in bootstrap there's this:

https://github.com/twbs/bootstrap-sass/blob/master/assets/stylesheets/bootstrap/_dropdowns.scss#L121

And the picker has the "dropdown-menu" class defined:

https://github.com/dangrossman/bootstrap-daterangepicker/blob/master/daterangepicker.js#L100

I think that .open on the body element is probably a bit too over-arching for an app that uses bootstrap and bootstrap components throughout. I'll create a new ticket to discuss this specifically though.

@wesleycho
Copy link
Contributor

Just a note, I have reverted the commit for this with 6f9f1fc - the previous behavior will take into effect, and this will require solution with user CSS. The issue will be tracked by #4466.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants