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

Update tab directive's select method to allow for event.stopPropagation() #5720

Closed
icfantv opened this issue Mar 31, 2016 · 5 comments
Closed

Comments

@icfantv
Copy link
Contributor

icfantv commented Mar 31, 2016

The current select method on the uibTab directive does not allow the user to stop the selection process via event.stopPropagation. We need to add an if block to check for this and not change tabs if the user's stopped the propagation.

Here's the relevant code: https://github.com/angular-ui/bootstrap/blob/master/src/tabs/tabs.js#L161-L173

We need to update the documentation as well.

This addresses comments/concerns in #2715, #4836, and #5716.

@icfantv icfantv changed the title Update tab directive select method to allow for event.stopPropagation() Update tab directive's select method to allow for event.stopPropagation() Mar 31, 2016
@icfantv icfantv self-assigned this Mar 31, 2016
@icfantv
Copy link
Contributor Author

icfantv commented Mar 31, 2016

So this turns out to be not as easy as we'd like. From a logic flow perspective, it is not right that the select method on the NEW tab be called to determine whether or not it's ok to leave the OLD tab.

A better solution, at the expense of API bloat, would be to add a beforeChange function to the tab (that defaults to noop) that would be called before the switching of the tabs is allowed. In that function we'd check the event.defaultPrevented property.

@icfantv
Copy link
Contributor Author

icfantv commented Mar 31, 2016

Currently looking at leveraging the deselect($event) method.

@icfantv
Copy link
Contributor Author

icfantv commented Mar 31, 2016

Using deselect($event) provides an acceptable solution. See this plunker for the demo - the "Static title" tab is not unselectable.

icfantv added a commit to icfantv/bootstrap that referenced this issue Mar 31, 2016
* add ability for user to prevent currently selected tab's deselction by
calling `$event.preventDefault()` in tab's `deselect` callback.

Fixes angular-ui#5720
Addresses angular-ui#2715, angular-ui#4836, and angular-ui#5716.
icfantv added a commit to icfantv/bootstrap that referenced this issue Mar 31, 2016
* add ability for user to prevent currently selected tab's deselection by
calling `$event.preventDefault()` in tab's `deselect` callback.

Fixes angular-ui#5720
Addresses angular-ui#2715, angular-ui#4836, and angular-ui#5716.
@daniel-jann
Copy link

The fix doesn't work for me: evt.defaultPrevented is undefined. I figured it's because I included the full jQuery library which redefines events with a isDefaultPrevented() instead of the standard defaultPrevented. I'm not sure where the fix should take place. On one hand I think it would be nice that jQuery follows standards but on the other hand I thought angular was supposed to work fine with the full jQuery library as is...

@icfantv
Copy link
Contributor Author

icfantv commented Apr 4, 2016

@daniel-jann, actually, you're sort of right. Angular found this issue as well and augmented their event object with the same method from jQuery (and even included a comment as to why):

function createEventHandler(element, events) {
  var eventHandler = function(event, type) {
    // jQuery specific api
    event.isDefaultPrevented = function() {
      return event.defaultPrevented;
    };
...

I'm going to reopen and fix.

@icfantv icfantv reopened this Apr 4, 2016
icfantv added a commit to icfantv/bootstrap that referenced this issue Apr 4, 2016
* switch event handler call to look at `event.isDefaultPrevented()` instead of
  `event.defaultPrevented` as angular has implemented that method on jqlite
  events.

Fixes angular-ui#5720
@icfantv icfantv closed this as completed in d767afb Apr 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants