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

[WIP] create a separate substate for editing (.edit) and create (.create) #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mangei
Copy link

@mangei mangei commented Sep 24, 2015

cleanup and better splitting of detail and edit mode
create state has to be registered before (!) the detail state, because it has a more specific url:
/new vs. /{:id}

THIS IS NOT READY TO BE MERGED, SOME PARTS ARE LOST DURING MIGRATION

OPEN FOR DISCUSSION

cleanup and better splitting of detail and edit mode
create state has to be registered before (!) the detail state, because it has a more specific url:
/new vs. /{:id}
@mangei
Copy link
Author

mangei commented Sep 24, 2015

Currently the Detail and Edit views share the same state and same controller. The goal is to separate these into different states. Even the create can have its own state.

Book.list
Book.create
Book.detail
Book.edit

The views would be more specific and would not contain view and edit code. The switch would just be a simple state change. The create can be a special form of the edit state, so it would also use the EditController, but with an empty entity.

We already made this work at our customer project and it worked fine. The code would in addition be more maintainable and easier to understand (no checks where am I, no detail copies, no view flags,...)

@tsalzinger
Copy link
Contributor

i completely support this change and would appreciate splitting the states / controllers / templates

one point for discussion though:

the detail and edit state both have to load the exact same data during there resolve, so basically they should probably have a common parent state

proposal:

Book {
   abstract: true,
   url: '/books'
}
Book.list {
   url: '',
   resolve: {/* listData */}
}
Book.new {
   url: '/new',
   resolve: {/* new model object */}
}
Book.detail {
   abstract: true,
   url: '/books/:id',
   resolve: {/* model data */}
}
Book.detail.show {
   url: ''
}
Book.detail.edit {
   url: '/edit'
}

@tsalzinger
Copy link
Contributor

in addition i would like to change the current implementation of the tabs, which should should be modeled as child states of the show state, but this is definitely a separate change, i just want to provide it as a note here

@mangei
Copy link
Author

mangei commented Sep 28, 2015

Thanks you for your input. I appreciate your support and will try to continue and finalize the implementation of it.

One simple remark: instead of Book.new, we should consider Book.create, because new is a keyword. This may not be a problem, but it could be. (it's the same with remove instead of delete)

@tsalzinger
Copy link
Contributor

i know what you mean with the keywords but the states are only strings, so there shouldn't be an issue

but i would even go a step further again and maybe resolve #44 first, which would allow users to configure all this by themselfes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants