-
Notifications
You must be signed in to change notification settings - Fork 20
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
Improve README file for beginners #76
Comments
Reference to #78 |
@visionarylab In order to make it clear, I created another issue for your problem. I will look into it asap. @zanybaka It seems like a nice feedback you made. I will try to read and respond to each part separately. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hey guys,
I've just tried to use all the described features and here are some ideas how to improve the readme.
1. Create Reducer functions
It is not so obvious without looking into your code that it is required to specify using static ReduxSimple.Reducers to use On.
It would be helpful to specify the full namespace ReduxSimple.Reducers.On in the examples.
2. Reducers on action
It seems the reducer is wrong as CurrentPage setter is not called.
Why don't you update it here?
I had to replace it with
3. Enable time travel
It's not so obvious how to track Redo action.
It would be helpful to specify it directly in the readme file or even to implement ObserveRedoneAction.
4.1 Entity management (in preview)
Could you publish the preview version as a separate alpha/beta package? something like ReduxSimple.Entity.dll
4.2 Then use the EntityAdapter in reducers
It is a potential error prone code imho.
a) the current UpsertOne impl requires the writable property Id in TodoItem class to be properly converted (would be nice to have just a field)
b) if you make a mistake in the anonymous class (name the property as ID instead of Id) you will receive the unfriendly message Value cannot be null. Parameter name: key' from the UpsertMany method.
It would be nice to raise some specific exceptions for such cases.
What you do think?
The text was updated successfully, but these errors were encountered: