Skip to content

Conversation

@jin521
Copy link

@jin521 jin521 commented May 29, 2018

No description provided.

vm.initialize = ->
vm.isLoading = false
if vm.isBillingShown()
vm.activeTab = 'billing'
Copy link

@yuritomanek yuritomanek May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jin521
Can this be moved into the setActiveTab() method to keep the logic all together?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Copy link
Contributor

@ouranos ouranos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to leverage the ui-router rather than parsing the state and setting the tab based on that.

You could either use:

Or do a bit more research on what's possible.

This link could be interesting to read to.

Also you only did this for 2 tabs out of the 4.

vm.activeTab = 'billing'
else
vm.activeTab = 'members'
#On refreshing the page it should stay on the same tab
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use one space between the leading # character of the comment and the text of the comment.

vm.initialize = ->
vm.isLoading = false
if vm.isBillingShown()
vm.activeTab = 'billing'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

#====================================
# Tab Management
#====================================
setActiveTab = () ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be made generic as there can be more tabs depending on settings and permission: billing, team, members, settings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants