search

rpocklin / ui-router-tabs

Idiot-proof tab panes with route support using Angular.js + Bootstrap 3 + UI Router
Other
244 stars 57 forks source link

child/nested tab targeting can lead to loss of parent tab highlighiting #55

Open skidvd opened 8 years ago

skidvd commented 8 years ago

I have a set of tabs with one tab containing another set of child/nested tabs. In my application, the child tabs can also be disabled via the associated attribute (unfortunately, do to other constraints preventing me from upgrading bootstrap in my application, I am not able to upgrade beyond v1.5 of ui-router-tabs at present).

I have a plunker demonstrating the issue at: http://plnkr.co/edit/UUcHHxlhPBbaWrMO6Srb?p=preview

In the plunker, Tab 3 is the one that has the child tabs and demonstrates the issue. The first child Tab (child 1) is disabled. In order to avoid landing the user that clicks on Tab 3 on a blank Tab, I set the Tab 3 route to 'main.tab3.child2' to target the first enabled tab and land them on the Child 2 tab instead. This works correctly and does maintain the correct Tab 3 parent tab highlighted.

However, if the user subsequently clicks on Child 3, the Tab 3 parent highlighting is lost. This is the issue I am trying to avoid (loss of parent tab active state highlighting).

If the Tab 3 route is only set to 'main.tab3', then the parent highlighting is not lost. However, the user will land on a blank Child 1 with it highlighted even though it is disabled.

Is there a workaround or better approach to ensure the user does not land on the blank Child 1/disabled tab and to also maintain the correct parent tab highlighting?

Thank you in advance for your help.

rpocklin commented 8 years ago

Thanks for pointing that out. It's a valid case you have - the problem is the code that detects if it's an ancestor is returning false since main.tab3.child2 is not an ancestor of main.tab3.child3. What it should probably do instead is compare them both starting with main.tab3. and return if they match on that or not.

Happy to accept a PR or will get to this sometime soon.

skidvd commented 8 years ago

I'd like to help with a PR. A bit swamped at present, but will see what I can do in the coming weeks if you don't get to it first.

rpocklin commented 8 years ago

Cool. Just need to come up with some logic that determines the ancestry correctly for both cases.

skidvd commented 8 years ago

I apologize for the lengthy delay.... I have been quite busy. However, I have taken some time today and made some progress on this....

As 1) this issue has to do with updating/maintaining the active status for a parent tab and 2) the default update_tabs() function only know about and deals with tabs at the present level, the first step became to gain visibility to the existence of parent tabs. I initially went down the path of require '?^^tabs' and ran into the fact that this unfortunately will NOT limit to parent only controller (as the documentation would lead us to believe) and will return the current level controller as well (see https://github.com/angular/angular.js/issues/4518). Including the current level controller in the results causes subsequent downstream problems when we attempt to update what we are hoping are (but in fact are not) parent level tabs. So, I resorted to traversing the DOM tree upwards from the current element and looking for a parent level 'tabs' element. This unfortunately ties things to a specific understanding of the DOM hiearchy created by the directive - I'm not happy with that.

Anyway, with the above significant caveats in mind, I now have a working updated 1.5 version I'd appreciate your initial thoughts on please. I'm pressed for time and I'm still not happy with the limitations, so I have not made this pretty nor progressed it to a PR. However you can see it working in action at: https://plnkr.co/edit/UUcHHxlhPBbaWrMO6Srb?p=preview Check out ui-router-tabs.1.5.0-EDIT.js there (as opposed to the default ui-router-tabs.1.5.0.js) to see the changes. Also, as a reminder, my present project is presently limited in such a way that we cannot progress beyond 1.5, so that is why I am working in that version.

I'd appreciate your thoughts, insights and ideas. Perhaps you have a better approach in mind - if so, I'd appreciate your perspective. If this spurs something in you, feel free to run with this as well as I'm not sure how long it may be until I may be able to further these efforts, but I will continue to try.

Thanks in advance!

rpocklin commented 7 years ago

Would be great to get this into a solution, feels like a loose end for this lib...

Endellur commented 7 years ago

Anyone have a work around for this? I'm hitting this as well. Thanks.