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

Closable tabs #166

Closed
bekos opened this issue Feb 21, 2013 · 12 comments
Closed

Closable tabs #166

bekos opened this issue Feb 21, 2013 · 12 comments

Comments

@bekos
Copy link
Contributor

bekos commented Feb 21, 2013

I think most of the applications need the ability to close a tab with an x square in their top-right corner.

So, a possible implementation should expose the removePane method to the $scope and the template should use an ng-show on a closable attribute from the pane to display or not the X button.

The problem here, is that Bootstrap's CSS does not support close buttons in tabs out of the box. So we need to style this button inline or provide another CSS file to change on hover the button's color , opacity, cursor etc

  1. Do you think this feature is worth it, in order to provide a PR?
  2. Can you think of a template to place the X button in a beautiful way?
  3. Any suggestion's about the CSS and on hover effects?

Thx

@sudhakar
Copy link

For the close button markup, you can probably use the BS markup used on the close button of alert

<button type="button" class="close">&times;</button>

It comes with hover styles as well.

@pkozlowski-opensource
Copy link
Member

@bekos Closable is not a bad idea, it could work as in the alert directive. The problem, though, is with static tabs. We would have to dynamically remove DOM and this makes me feel uneasy.

Feel free to play with the idea, on our side we need to merge other, existing PRs first.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 22, 2013

Why not just use ng-repeat and splice the tab from the array?

@bekos
Copy link
Contributor Author

bekos commented Feb 22, 2013

@ajoslin This way i have to provide a custom template in order to show the X button for a simple fetaure that is provided by default in other libraries like Jquery UI and also we must find a way to handle static tabs which don't exist in the controller's scope array.

By the way, playing around with this, i have a problem when removing the div (content) element from a static tab, this triggers the destroy event for every pane. Any idea why this is happening?

@bekos
Copy link
Contributor Author

bekos commented Feb 23, 2013

I don't know if i should open a new issue, but while working with tabs, i feel that the support for static tabs makes things a bit confusing. For example, lets say i want to display first a static tab, some dynamic tabs based on some preferences then, and a lastly another static tab. Then, simply thinking (not an expert in angular), i would write in my HTML something like this:

< tabs > < pane heading="Static title">Static content < pane ng-repeat="pane in panes" heading="{{pane.title}}" active="pane.active">{{pane.content}} < pane heading="Last Tab">Static content 2 < /tabs >

But the actual output is first all the static tabs and then all the dynamic ones (it 's obvious :-) ).
Also i see in the tests that they don't mix static and dynamic tabs, and the remove function is not tested for static tabs either.

So, I think that using angular just for html encapsulation is bit of abusing, and the way it should go, is to force users to use scope objects to define all their entities. So, in my opinion again, a syntax like:

< tabs panes="panes"> ...Probably here some HTML elements that we want as contents in some elements... (not necessarily) < /tabs>

and the controller

$scope.panes = [
{heading: 'H1', content: 'bla bla'}
{heading: 'H2', contentEl: '#id'} // inside tab directive
{heading: 'H3', contentUrl: '... a partial'}
{heading: 'H4', content: 'bla bla', active: true} // default selected
]

may be a better approach, in order to be able to implement and other features in a cleaner & more robust way.
What is your opinion?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 28, 2013

We are working a <pane-heading> pull request #151 right now which would do this.

For your case it would look like:

<tabs>
  <pane ng-repeat="p in panes">
    <pane-heading>
      {{p.title}} <a ng-click="removePane(p)">[x]</a>
    </pane-heading>
    {{p.content}}
  </pane>
</tabs>

@pkozlowski-opensource
Copy link
Member

@bekos Since #151 was merged you should be able to add icons to the tab header now.

Going to close this one for now, please re-open if things are still missing.

@thiloplanz
Copy link

Has #151 really been merged?

What would be the syntax to add icons to the tab header now?

@bekos
Copy link
Contributor Author

bekos commented Apr 5, 2013

It 's not merged yet. There will be no specific syntax for "close" icons, but you will be able to write vanilla HTML inside your tab-heading.

@thiloplanz
Copy link

So both this and #151 are really dupes of #124 ?

@bekos
Copy link
Contributor Author

bekos commented Apr 5, 2013

All these issues are solved in the same way. This one started as an idea to provide a default close icon like the way we do in alert, but since you can right any HTML in your heading and call any scope function you want, it was decided not to implement it inside the directive's core.

@MaximilianoRicoTabo
Copy link

++ to final choice. I'll add a delete icon instead of close icon and delete a model instead of just close it.

codedogfish pushed a commit to codedogfish/angular-ui-bootstrap that referenced this issue Sep 15, 2015
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

6 participants