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

Improve README file for beginners #76

Open
zanybaka opened this issue Sep 12, 2020 · 2 comments
Open

Improve README file for beginners #76

zanybaka opened this issue Sep 12, 2020 · 2 comments

Comments

@zanybaka
Copy link

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.

public static class Reducers
{
    public static IEnumerable<On<RootState>> CreateReducers()
    {
        return new List<On<RootState>>
        {
            ReduxSimple.Reducers.On<NavigateAction, RootState>(

2. Reducers on action

It seems the reducer is wrong as CurrentPage setter is not called.
Why don't you update it here?

You can define a list of On functions where at least one action can be triggered.

return new List<On<RootState>>
{
    On<NavigateAction, RootState>(
        (state, action) => state.With(new { Pages = state.Pages.Add(action.PageName) })
    ),

I had to replace it with

On<NavigateAction, RootState>(
                    (state, action) => state.With(new
                    {
                        CurrentPage = action.PageName,
                        Pages       = state.Pages.Add(action.PageName)
                    })),

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.

// Undo
App.Store.ObserveUndoneAction().Subscribe(o =>
{
    ...
});

// Redo
App.Store.ObserveAction(ActionOriginFilter.Redone).Subscribe(o =>
{
    ...
});

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

On<CompleteTodoItemAction, TodoListState>(
    (state, action) =>
    {
        return state.With(new
        {
            Items = TodoItemAdapter.UpsertOne(new { action.Id, Completed = true }, state.Items)
        });
    }
)

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?

@visionarylab
Copy link

visionarylab commented Sep 25, 2020

Reference to #78

@Odonno
Copy link
Owner

Odonno commented Oct 7, 2020

@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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants