-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add optional props to ICard.commit #147
Conversation
shangzhel
commented
Oct 9, 2021
- Unblocks Add favorite cards feature #12
- The following code is now valid
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.
When the user is editing something in the card detail, and the same card is marked as favorite, the changes are reverted
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.
Thank you
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.
Soz for more requested changes.
But I noticed that cardDetail's unsaved changes get commited alongside favorite, is it possible to commit something while retaining the unsaved changes?
Don't care about keys here.
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.
Normal commits after updates now no longer works.
I think:
if something is given to the commit function, then it should only commit what's given.
if nothing is given to the commit function, then it should commits what's been given in update().
override should persist until commit is called with with no arguments.
Alternatively you can make another function called directCommit() or something like that which only commits what's given to it. And commit will simply remain as it is on master.
That is what it is supposed to do. |
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.
lgtm.
Thanks again, I hope I've tested every scenario this time.