Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tabs: don't transclude content initially to check for nav-view #730

Closed
ajoslin opened this issue Mar 5, 2014 · 8 comments
Closed

Tabs: don't transclude content initially to check for nav-view #730

ajoslin opened this issue Mar 5, 2014 · 8 comments
Assignees
Milestone

Comments

@ajoslin
Copy link
Contributor

ajoslin commented Mar 5, 2014

How we we find out if there's a nav-view in the tabs is to transclude the content initially, but do nothing with it. This could have side effects that we aren't prepared to deal with (eg creating a controller for a moment could set a variable or basically anything).

It also causes this error:

TypeError: Object #<Comment> has no method 'setAttribute'
    at forEach.attr (http://code.ionicframework.com/nightly/js/angular/angular.js:2462:15)
    at Object.JQLite.(anonymous function) [as attr] (http://code.ionicframework.com/nightly/js/angular/angular.js:2569:9)
    at Object.Attributes.$set (http://code.ionicframework.com/nightly/js/angular/angular.js:5430:28)
    at Object.interpolateFnWatchAction [as fn] (http://code.ionicframework.com/nightly/js/angular/angular.js:6534:28)
    at Scope.$digest (http://code.ionicframework.com/nightly/js/angular/angular.js:11802:29)
    at Scope.$apply (http://code.ionicframework.com/nightly/js/angular/angular.js:12055:24)
    at http://code.ionicframework.com/nightly/js/angular/angular.js:1301:15
    at Object.invoke (http://code.ionicframework.com/nightly/js/angular/angular.js:3704:17)
    at doBootstrap (http://code.ionicframework.com/nightly/js/angular/angular.js:1299:14)
    at bootstrap (http://code.ionicframework.com/nightly/js/angular/angular.js:1313:12)
@mlynch
Copy link
Contributor

mlynch commented Mar 5, 2014

I guess the question is why do the tabs have anything to do with the nav system? I know the tabs rely on the nav stuff, but we should think about how to decouple them as much as possible.

@ajoslin
Copy link
Contributor Author

ajoslin commented Mar 5, 2014

Currently, we simply need to associate a tab with any nav-view inside it, so we can select the tab if the url switches to that navView.

The better way to do this would be to put an attribute on the tab saying which nav-view it is associated with, so we don't have to initially transclude the content and check for a nav-view.

Alternatively, we could do our own transclusion (not using angular's default, which lets us know the html of the element) and just check for a nav-view in the HTML string during the compile phase. That might be the best?

@ajoslin
Copy link
Contributor Author

ajoslin commented Mar 5, 2014

OK, that works. We can simply not use angular transclusion.

  1. In the compile phase, we remove the contents of the <ion-tab> from the dom (so they aren't compiled)
  2. We search the contents for an ion-nav-view and get the name attribute from it, then set things accordingly.
  3. Now whenever we want to insert tab's contents into the DOM, we simply clone our saved contents, insert, and $compile

on my fork: ajoslin@7d33e4f#diff-f9bd4c766836959edb16dde8bb0ebb7cL153

@adamdbradley
Copy link
Contributor

To answer @mlynch's question, if a URL is for a tab that's not the first tab, when that URL is the initial load it needs to know to open up the correct tab. Whatever the best most efficient way to accomplish this I'm all for.

@adamdbradley adamdbradley added this to the 1.0 Beta2 milestone Mar 5, 2014
@mlynch
Copy link
Contributor

mlynch commented Mar 5, 2014

What if the nav-view inside the tab sends a "register" event up the scope
tree, and the tab can "confirm" that it is the owner of it?

On Wed, Mar 5, 2014 at 12:39 PM, Adam Bradley [email protected]:

To answer @mlynch https://github.com/mlynch's question, if a URL is for
a tab that's not the first tab, when that URL is the initial load it needs
to know to open up the correct tab. Whatever the best most efficient way to
accomplish this I'm all for.


Reply to this email directly or view it on GitHubhttps://github.com//issues/730#issuecomment-36777079
.

@ajoslin
Copy link
Contributor Author

ajoslin commented Mar 5, 2014

The problem is a tabs content is not put into the Dom until it is selected,
but we may have to select if it contains a nav view that matches the URL.

So we can check the nav views name without rendering or compiling the
content.
On Mar 5, 2014 11:51 AM, "Max Lynch" [email protected] wrote:

What if the nav-view inside the tab sends a "register" event up the scope
tree, and the tab can "confirm" that it is the owner of it?

On Wed, Mar 5, 2014 at 12:39 PM, Adam Bradley [email protected]:

To answer @mlynch https://github.com/mlynch's question, if a URL is
for
a tab that's not the first tab, when that URL is the initial load it
needs
to know to open up the correct tab. Whatever the best most efficient way
to
accomplish this I'm all for.

Reply to this email directly or view it on GitHub<
https://github.com/driftyco/ionic/issues/730#issuecomment-36777079>
.

Reply to this email directly or view it on GitHubhttps://github.com//issues/730#issuecomment-36778460
.

@mlynch
Copy link
Contributor

mlynch commented Mar 5, 2014

I see. So the other question is, is it worth it to enable this feature, which is really only relevant during development?

One alternative from a developers point of view is to control the experience through their own code. For example, if you wanted to use a URL routing system, you might just listen for a URL, and what that is hit, perform some code to select that tab, like this pseudocode:

$urlRouter.on('/tab/cats', function() {
  selectTab(1);
}); 

When in doubt, I'd perfer doing less, assuming less, and making it easier for the developer to control the experience.

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 6, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 6, 2018
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

3 participants