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

feat(): dropdown-toggle rewrite #284

Closed
shaungrady opened this issue Mar 30, 2013 · 28 comments
Closed

feat(): dropdown-toggle rewrite #284

shaungrady opened this issue Mar 30, 2013 · 28 comments

Comments

@shaungrady
Copy link
Contributor

@bekos @pkozlowski-opensource @ajoslin, here's what I'm thinking about for a refactored dropdownToggle directive. First, I'd propose renaming it to simply dropdown. The markup would be very simple:

<button class="dropdown" label="Dropdown Button <span class='caret'></span>">
  <ul>
    <li><a href="#">Do</a></li>
    <li><a href="#">Re</a></li>
    <li class="divider"></li>
    <li><a href="#">Mi</a></li>
  </ul>
</button>

When the dropdown directive is compiled, it would move the ul to the outside of the button (so it's a sibling) and set a style of display: none, then the innerHTML of the button would be set to the label attr.

When the button is clicked, the ul will have the display: none style unset and the button will receive a class of active (could be changed via activeclass attr, or config?)

I think this is a simple and flexible solution, and it's all self-contained. The only downside to this approach is that it assumes the dropdown element is wrapped in a relatively positioned element, e.g., a div.btn-group.

What are your thoughts?

@shaungrady
Copy link
Contributor Author

I forgot to add, there'd be no isolate scope or transclusion happening.

@joshdmiller
Copy link
Contributor

I've only been following this conversation passively, but I'm hoping to get more involved.

My issue here is that it's very Bootstrap-specific. It seems like we would have to rewrite the plugin in order to use something else (e.g. Foundation).

@shaungrady
Copy link
Contributor Author

It's not really, though. If it's desired, the directive could automatically apply position: relative to the parent element and position: absolute to the ul, along with maybe a top and left. The only part of this concept that relies on Bootstrap is giving the button a class of active when the ul is being displayed—and that can easily be configurable.

I'm not familiar with Foundation and I did some Googling, but I wasn't able to find an example of a dropdown menu using Foundation—can you point me to a specific example?

@shaungrady
Copy link
Contributor Author

Ahh, I found Foundation's docs on it. They do dropdowns strangely—not a fan. Regardless, making it compatible with Foundation probably wouldn't be too much work. The biggest hurdle I can see is getting accurate positioning. That aside, Foundation and Bootstrap do not behave the same way, especially in the case of the split button: Bootstrap's dropdown is positioned relative to parent while Foundation's dropdown is positioned relative to the toggler. That could be reconciled by allowing the user to specify whether they would like the dropdown positioned relative to parent or button.

@joshdmiller
Copy link
Contributor

UI Bootstrap has a long-term goal to support any framework, which is why we've been trying really hard to ensure none of the components include knowledge of the DOM structure or CSS classes from within the directive (though it's not perfect yet), but instead place all of that in the template.

I agree that dropdowns are a tricky case, but I feel we should try to find a solution that doesn't tie us to any framework and would hopefully rely on templates for everything DOM- or style-related.

@shaungrady
Copy link
Contributor Author

The frameworkless path would be the Foundation method of absolutely positioning the dropdown as a direct child of the body elem. The markup in the original post would still work for that path. So then the user would need to configure if they want the dropdown positioned relative to the toggler's parent or the toggler itself--the Bootstrap style or the Foundation style, respectively.

@bekos
Copy link
Contributor

bekos commented Mar 30, 2013

@angular-ui/bootstrap Why we can't use something like popover's code with a <ul> content and an additional bind for document clicks in order to close the popover? This way we can open the dropdown in many directions and with animation. This is more like foundation's implementation of dropdown that can hold also content.

Bootstrap's feaures for the dropdown are keyboard navigation and close on Esc. I think a refactor should address this ones also.

@shaungrady
Copy link
Contributor Author

Good idea, looks like a lot can be borrowed from popover. I agree that the refactor should support keyboard navigation/dismissal.

@ajoslin
Copy link
Contributor

ajoslin commented Mar 30, 2013

Yeah you're right, it is a lot like popover. Another thing: I think we should have no opinion about what element opens it - we should just have it have a variable for being shown/hidden.

Or ... do we need to know something for the sake of positioning?

@bekos
Copy link
Contributor

bekos commented Mar 30, 2013

@ajoslin I agree with the variable, so that we can programmatically open / close the dropdown. If we follow bootstrap's html I don't think there will be a problem with positioning.

@shaungrady
Copy link
Contributor Author

The toggler and the menu are still linked in that the menu has to be positioned relative to the toggler. Following Bootstrap's HTML as far as including the btn-group wrapper in the directive would bring back the problem of maintaining flexibility of implementation.

I'm not sure what you mean @ajoslin when you talk about abstracting it down to a variable—what would the use case be for that? Having many togglers to one menu? How would that relationship be defined?

@bekos
Copy link
Contributor

bekos commented Mar 30, 2013

@shaungrady I think @ajoslin proposes the dropdown directive will $watch a variable in order to be visible or hidden. This way we can toggle the dropdown from anywhere. The toggler as you mention it, will be another way to change the value of this variable.

Also, the toggler will be used to position the dropdown element.

@ajoslin
Copy link
Contributor

ajoslin commented Mar 30, 2013

@shaungrady yeah, as @bekos said.

You could do btn-group like this:

<div class="btn-group" ng-class="{toggle: showDropdown}">
  <button class="btn">Hello</button>
  <button class="btn dropdown-toggle" ng-cilck="showDropdown = !showDropdown">
    <span class="caret"></span>
  </button>
  <!-- compiles to <ul> element -->
  <dropdown is-open="showDropdown"> 
    <li ng-repeat="opt in options">{{opt}}</li>
  </ul>
</div>

It wouldn't even require a new scope - it would just use $parse to set attrs.isOpen to false whenever the user clicks outside.

This way allows you to do weird dropdown buttons that aren't sibling elements with their openers. Eg:

<div class="things">
  <dropdown is-open="showMe">
    <li>Option</li>
  </dropdown>
</div>
<button class="btn" ng-click="showMe=!showMe">Toggle Dropdown</button>

@shaungrady
Copy link
Contributor Author

So the toggler and menu would be defined separately, and you'd have to give each menu a unique identifier that each toggler would use to reference a dropdown. Sounds fragile, and I'm not sure that use case would be very common. You lose the convenience to defining the dropdown in one place and not having to worry about how it works.

A DRY approach of defining one menu to many togglers could be achieved by defining the menu in the scope and referencing it in a toggler attr, or by passing a menu template to the toggler to be compiled with the toggler.

@shaungrady
Copy link
Contributor Author

I think I've got a good idea for how I envision it working—I'll start work on my potential solution when I've got spare time and show it to you guys for feedback.

@ajoslin
Copy link
Contributor

ajoslin commented Mar 30, 2013

You may be right... that non-sibling elements will never be used. But we still don't want to enforce dom structure.

Why do we need a unique id for each dropdown? Can't we just let them be positioned as they normally would?

@shaungrady
Copy link
Contributor Author

The "unique ID" in your second example would be showMe, which would allow the toggler to affect the dropdown visibility. Using your example, a unique var would need to be chosen for each menu/toggler set. I think that kind of detail should be altogether hidden from the user—it shouldn't be something they concern themselves with.

@ajoslin
Copy link
Contributor

ajoslin commented Mar 30, 2013

Ah, you're right. Ok I'm not convinced on my idea. What does everyone else think?

@ajoslin
Copy link
Contributor

ajoslin commented Mar 30, 2013

I am actually starting to like your new syntax... People can use a popover if they want weird stuff like what I'm describing.

we don't usually like HTML in attributes though. For your original example, couldn't just have the label be text of the button?

<button class="dropdown">
  Dropdown Button <span class='caret'></span>  
  <ul>
    <li><a href="#">Do</a></li>
    <li><a href="#">Re</a></li>
    <li class="divider"></li>
    <li><a href="#">Mi</a></li>
  </ul>
</button>

@shaungrady
Copy link
Contributor Author

Here's my initial commit, see the commit message for details.Everything's working, but no keydown listener yet.
I'd love to have feedback on it: 57e8685

@shaungrady
Copy link
Contributor Author

Also forgot to mention that, currently, the dropdown menu is absolutely positioned relative to the directive element's parent. I still need to add the option to configure which element the dropdown is positioned relative to—whether the user would like it positioned relative to the parent (bootstrap's behavior) or relative to the toggler (Foundation's behavior).

@ajoslin
Copy link
Contributor

ajoslin commented May 22, 2013

.... two months later .... sorry shaun :-)

I like your implementation! @shaungrady would you like to open a PR with that commit and tests?

If you have moved past this, I can try that.

@shaungrady
Copy link
Contributor Author

Life has kept me quite busy lately, so I haven't done a whole lot more work on this. I've been using a revised form of this directive in my 9-to-5. Having the ul inside the button element is invalid HTML, and some browsers (IE) actually pull it out of the button and make it a sibling. So I've changed it a bit and required the dropdown portion to be the next sibling to the directive element. I can't think of any other way to do it that wouldn't require referencing an dropdown menu's ID or what not from the toggler.

@ajoslin
Copy link
Contributor

ajoslin commented Jul 27, 2013

I'm thinking perhaps:

<dropdown-menu>
  <a class="btn dropdown-toggle" dropdown-toggle ng-model="dropIsOpen"> 
    Dropdown <i class="caret"></i>
  </a>
  <ul>
    <li>Option</li>
    <li>Option</li>
  </ul>
</dropdown-menu>

The dropdown-menu element itself can be removed from the dom and the insides placed as siblings as you said, shaun. The parent is just to link the two together: watch ngModel for when it opens, then bind $document click events to close it. This also lets the user use other events like ng-mouseenter and ng-mouseleave to open/close the dropdown.

The dropdown-menu parent will know every child that isn't a dropdown-toggle is the dropdown's content.

@ajoslin ajoslin closed this as completed Jul 27, 2013
@ajoslin ajoslin reopened this Jul 27, 2013
@ajoslin
Copy link
Contributor

ajoslin commented Jul 27, 2013

oops :-)

@shaungrady
Copy link
Contributor Author

Interesting approach Andy. A couple thoughts:

I'm partial to the idea that the menu element be placed as a child of the body—or, at the very least, a child of the nearest offset parent. The concern being that the menu element can interfere with sibling selector styles—e.g., if you have a group of buttons that aren't rounded when placed beside each other, a menu element placed between them would break that style. Additionally, if the menu element is moved to be a direct child of the body it wouldn't have the possibility of being clipped by an overflow: hidden parent of the toggler.

Second, I think linking the visibility state of the dropdown to the model should be optional. If you've got tabular data and you need every ngRepeated row to have a dropdown menu associated with it, you're not going to want to assign a model value to each one.

@ajoslin
Copy link
Contributor

ajoslin commented Jul 27, 2013

So dropdown menu could really just be a popover with a different template? :-)

@rtpm
Copy link

rtpm commented Oct 14, 2013

Any news on dropdown directive ? :-)

Currently menu is closed by watching for $location.path() but my menu is located in a scope outside ng-view,
so I have to listen for $routeChangeStart event.

bekos added a commit to bekos/bootstrap that referenced this issue Jan 19, 2014
bekos added a commit to bekos/bootstrap that referenced this issue Jan 20, 2014
@bekos bekos closed this as completed in ae31079 Jan 21, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants