-
Notifications
You must be signed in to change notification settings - Fork 399
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
Redux support #8
Conversation
… well combined with redux
const loadMember = (id : number) => { | ||
return { | ||
type: 'MEMBER_LOAD' | ||
,id: id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually it puts the comma at the end of the previous line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to find an standard way, I like to place it at the beginning, because when I remove a line sometimes I get a mess (error).
var memberIdNumber : number = parseInt(memberId); | ||
this.props.loadMember(memberIdNumber); | ||
} else { | ||
this.props.newMember(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that rename to 'initializeNewMember' is more explicit.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea
if(errorsSave.isEntityValid == true) { | ||
MemberAPI.saveAuthor(state.member); | ||
// TODO: pending clone member object !! (keep state inmmutable) | ||
newState = objectAssign({}, state, {isValid: true, saveCompleted: true, errors: new MemberFormErrors()}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are you using state.isValid property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that you only need to update the field saveCompleted because your are initializing errors field in 'MEMBER_LOAD' case
newState = objectAssign({}, state, {saveCompleted: true});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true if we are able to save all that is not needed
let newMember : MemberEntity = objectAssign({}, state.member, {}); | ||
newMember[fieldName] = value; | ||
|
||
newState = objectAssign({}, state, {member: newMember, dirty: true}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are you using state.dirty property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's another point I would leave it as it is now, and later on we add e.g. a function that ask you when you navigate away fro mthe page, if there are pending changes to save.
Added sample 09, redux support, using fake api (sync).