From 5f2f4c38528d8773b970e5e09d9d67789e2b1672 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 23 Dec 2014 00:04:44 +0100 Subject: [PATCH] First stab at fixing weird view errors This commit starts refactoring of one of the remaining areas where we do weird tricks to get the desired behaviour. Namely, we were treating "my_repositories" and "recent" not as individual routes with separate URLs, but only different states on the repos controller. Such approach leads to various problem with connecting outlets on rerenders (ie. we don't explicitly connect outlets when changing from one view to another programatically). A new cleaner way is to change both tabs into routes. --- assets/scripts/app/controllers/repos.coffee | 20 ------- assets/scripts/app/routes.coffee | 53 ++++++++++++++++--- assets/scripts/app/templates/layouts/top.hbs | 4 +- .../scripts/app/templates/repos/list/tabs.hbs | 4 +- assets/scripts/app/views.coffee | 2 +- assets/scripts/app/views/log.coffee | 2 +- assets/scripts/app/views/repo/show.coffee | 18 ------- 7 files changed, 53 insertions(+), 50 deletions(-) diff --git a/assets/scripts/app/controllers/repos.coffee b/assets/scripts/app/controllers/repos.coffee index 7935067b..f1953204 100644 --- a/assets/scripts/app/controllers/repos.coffee +++ b/assets/scripts/app/controllers/repos.coffee @@ -3,23 +3,8 @@ require 'travis/limited_array' Travis.ReposController = Ember.ArrayController.extend actions: activate: (name) -> - @transitionToRoot() @activate(name) - defaultTab: ( -> - if @get('currentUser.id') - 'owned' - else - 'recent' - ).property('currentUser.id') - - currentUserIdDidChange: (-> - if @get('currentUser.id') - @activate('owned') - else if @get('tab') == 'owned' - @activate('recent') - ).observes('currentUser.id') - tabOrIsLoadedDidChange: (-> @possiblyRedirectToGettingStartedPage() ).observes('isLoaded', 'tab', 'length') @@ -58,13 +43,8 @@ Travis.ReposController = Ember.ArrayController.extend if content = @get('content') content.forEach (r) -> r.updateTimes() - transitionToRoot: -> - @container.lookup('router:main').send('renderDefaultTemplate') - @container.lookup('router:main').transitionTo('index.current') - activate: (tab, params) -> @set('sortProperties', ['sortOrder']) - tab ||= @get('defaultTab') @set('tab', tab) this["view#{$.camelize(tab)}"](params) diff --git a/assets/scripts/app/routes.coffee b/assets/scripts/app/routes.coffee index 2017cfaf..60717a82 100644 --- a/assets/scripts/app/routes.coffee +++ b/assets/scripts/app/routes.coffee @@ -43,14 +43,17 @@ Travis.ApplicationRoute = Travis.Route.extend if transition = @auth.get('afterSignInTransition') @auth.set('afterSignInTransition', null) transition.retry() + else + @transitionTo('index') afterSignOut: -> - @transitionTo('index.current') + @transitionTo('index') Travis.Router.map -> @resource 'index', path: '/', -> @resource 'getting_started' - @route 'current', path: '/' + @route 'recent', path: '/recent' + @route 'my_repositories', path: '/my_repositories' @resource 'repo', path: '/:owner/:name', -> @route 'index', path: '/' @resource 'build', path: '/builds/:build_id' @@ -130,7 +133,8 @@ Travis.GettingStartedRoute = Travis.Route.extend Travis.SimpleLayoutRoute = Travis.Route.extend setupController: -> $('body').attr('id', 'home') - @container.lookup('controller:repos').activate() + toActivate = if @signedIn() then 'owned' else 'recent' + @container.lookup('controller:repos').activate(toActivate) @_super.apply(this, arguments) renderTemplate: -> @@ -148,7 +152,7 @@ Travis.InsufficientOauthPermissionsRoute = Travis.SimpleLayoutRoute.extend existingUser = document.location.hash.match(/#existing[_-]user/) controller.set('existingUser', existingUser) -Travis.IndexCurrentRoute = Travis.Route.extend +Travis.IndexMyRepositoriesRoute = Travis.Route.extend renderTemplate: -> @render 'repo' @render 'build', into: 'repo' @@ -158,6 +162,34 @@ Travis.IndexCurrentRoute = Travis.Route.extend @currentRepoDidChange() @controllerFor('repo').activate('index') + @controllerFor('repos').activate('owned') + @controllerFor('repos').addObserver('firstObject', this, 'currentRepoDidChange') + + afterModel: -> + @controllerFor('repos').possiblyRedirectToGettingStartedPage() + + deactivate: -> + @controllerFor('repos').removeObserver('firstObject', this, 'currentRepoDidChange') + + currentRepoDidChange: -> + @controllerFor('repo').set('repo', @controllerFor('repos').get('firstObject')) + + actions: + redirectToGettingStarted: -> + @transitionTo('getting_started') + + +Travis.IndexRecentRoute = Travis.Route.extend + renderTemplate: -> + @render 'repo' + @render 'build', into: 'repo' + + setupController: -> + @_super.apply this, arguments + @currentRepoDidChange() + + @controllerFor('repo').activate('index') + @controllerFor('repos').activate('recent') @controllerFor('repos').addObserver('firstObject', this, 'currentRepoDidChange') afterModel: -> @@ -293,6 +325,13 @@ Travis.RepoRoute = Travis.Route.extend # bubble to the top return true +# Obviously Index route should be renamed to something +# like "main" or "home" +Travis.IndexIndexRoute = Travis.Route.extend + redirect: -> + target = if @signedIn() then 'my_repositories' else 'recent' + @transitionTo("index.#{target}") + Travis.IndexRoute = Travis.Route.extend renderTemplate: -> $('body').attr('id', 'home') @@ -302,7 +341,9 @@ Travis.IndexRoute = Travis.Route.extend @render 'repos', outlet: 'left', into: 'index' setupController: (controller)-> - @container.lookup('controller:repos').activate() + # TODO: this is redundant with my_repositories and recent routes + toActivate = if @signedIn() then 'owned' else 'recent' + @container.lookup('controller:repos').activate(toActivate) Travis.StatsRoute = Travis.Route.extend renderTemplate: -> @@ -377,7 +418,7 @@ Travis.AuthRoute = Travis.Route.extend actions: afterSignIn: -> - @transitionTo('index.current') + @transitionTo('index') return true Travis.SettingsRoute = Travis.Route.extend diff --git a/assets/scripts/app/templates/layouts/top.hbs b/assets/scripts/app/templates/layouts/top.hbs index bc9c1e53..3ddc2e88 100644 --- a/assets/scripts/app/templates/layouts/top.hbs +++ b/assets/scripts/app/templates/layouts/top.hbs @@ -1,4 +1,4 @@ -{{#link-to "index.current"}} +{{#link-to "index"}}

Travis

@@ -6,7 +6,7 @@