Storytel / react-redux-spinner

An automatic spinner for react and redux
https://storytel.github.io/react-redux-spinner/
MIT License
81 stars 13 forks source link

multiple redux spinners #24

Open mstruensee opened 6 years ago

mstruensee commented 6 years ago

Is there any way to have multiple instance of this by using the curry design? or any design?

What I am trying to do is, basically have a bar per bootstrap card, vs 1 bar across the entire page. I want to have the ability to start n stop each one individually based on that reducer, etc...

Is this at all possible with the current design?

noseglid commented 5 years ago

I'm sorry for taking ages to reply to this. I must've missed it when you made it and haven't checked back since.

Yes, that should be feasible. The reducer is already configurable so we could have multiple reducers at separate paths. We'd need to do something with the actions (since they only act on a specific key) and make them configurable too. Then we'll make the component configurable too, essentially telling it which part of the redux state to look at.

Is this something still interesting, or have you moved on to other things?

kingo999 commented 5 years ago

I think this may be related to my request/issue @noseglid @mstruensee :-)

mstruensee commented 5 years ago

I actually forgot about this since I switched teams, but yes, I would still like to get this working! I will give this a go again with new information. Any additional info would be greatly appreciated.

Thanks.

kingo999 commented 5 years ago

Hi Thanks for the reply! Ideally what would be ideal would be to assign an Id to a spinner so it cant have duplicates. I might try have a go at this myself if I get some time and submit a PR.

mstruensee commented 5 years ago

I get the concept of configurablePendingReducer ... how do I add multiple throughout the app and have the correct to the correct reducer (key) ?

Per original comment, example is a app level across the entire page, and then one inside a . So have 100 cards, each have their own, and the app level one is done with all 100 cards are done ....

i think we need this to be configurable in spinner.js ... const diff = store.getState().**pendingTasks** .. would be easy to get that from the config being passed in ya?

then I think it should pan out ... const pendingTaskSpecificName = configurablePendingTaskReducer({ actionKeyPath: ["meta", "specificName"] })

export default combineReducers({ pendingTasks: pendingTasksReducer, specificName: pendingTaskSpecificName, }) const diff = store.getState()[config.reducerName || "pendingTasks"]

noseglid commented 5 years ago

You're onto it there. I think these things must be done;

I think the difficult thing here is to synchronize the reducer, action and the component. What should that property the component gets look like? Ideally we'd want the library to expose something which generates this in a deterministic way.

mstruensee commented 5 years ago

Would we need to change anything except having the option of setting the reducer name? If that exists, I can add 20 <Spinner config={{reducerName: "x"}}> on the page, all then everything can be handled like normal ...

inside ur action just put your name ? `store.dispatch({ type: 'ANY_OF_YOUR_ACTION_TYPES'

});`

everything still can flow like normal, with the key option, the task count, etc ... because they are checking the task count for that specific reducer ...

I will have to do some testing.

mstruensee commented 5 years ago

I see what you're referring to, it is not just a simple change in 1 spot lol ...

noseglid commented 5 years ago

No I think it requires a simple change in 3 spots :)

Let me know if you're giving this a stab, otherwise I'll see if I can get some time to get this done.

mstruensee commented 5 years ago

I tried that and I didn't get the result I was expecting, still testing ...

mstruensee commented 5 years ago

NProgress puts the div on the tree using id="nprogress" .. I think this is preventing me from having 2 on the same page.

mstruensee commented 5 years ago

here is a fork for nprogress which they made for multi ... https://github.com/RamyRais/multi-nprogress#readme

there is an issue though, with having one inside another, I commented, hopefully they can fix it. If/once fixed ... you can use that. I forked yours and have it done (I copied the nprogress.js file and committed it), but I still need to attach the reducer name to the config. Will try that some tomorrow if I have time.

noseglid commented 5 years ago

NProgress puts the div on the tree using id="nprogress" .. I think this is preventing me from having 2 on the same page.

Oh yeah, that's gonna be an issue.

At some point I've been thinking about rolling our own progress bar so we can scrap the dependency of nprogress. This would've been easier.

I'm not sure I'm comfortable changing the dependency to multi-progress. Seems very tiny and i'm not sure of the maintenence situation

mstruensee commented 5 years ago

I was thinking the same thing, My fork has the nprogress multi piece working .. you want to check that out? lmk what you think/plans, I can help if needed ...

mstruensee commented 5 years ago

I have a hacked version working ... but I do not like it ... I envision something like this ...

`store.dispatch({ type: "FETCHING_ASYNC_DATA",

        [anotherTask]: begin
    })

store.dispatch({ type: "FETCHING_ASYNC_DATA",

        [thirdTask]: begin
    })

` ^^ this will have 2 task started at the "body" level, and each individual "container loader" will have 1 ... then when a container finishes, it goes away, but the "body" level is down to 1.

the only possible idea I have is to curry the actionKey ... something like

`store.dispatch({ type: "FETCHING_ASYNC_DATA",

        [pendingTask('container1')]: begin
    })

store.dispatch({ type: "FETCHING_ASYNC_DATA",

        [pendingTask('container2')]: begin
    })

`

But the problem is typing the reducer name in combineReducers to the action type (pendingTask, [anotherTask]) ... naming the reducer pendingTask it gives the illusion that pendingTask is somehow linking to it (with the names) but its not in reality... as the reducer name is pendingTask and pendingTask is "@@react-redux-spinner/pending-task" ...

Not sure how to make it dynamic where the evaluated [anotherTask] points to anotherTask reducer ... ideas? see here for working example

To me, what I want is too specific for dynamic/configurability ...

mstruensee commented 5 years ago

what are your thoughts?

noseglid commented 5 years ago

If we're doing something like pendingTask('container1') - we might just as well use the string straight up.

Hmm, What if we provided an array to the configurablePendingTasksReducer which all the strings it should listen to, and it would keep one integer in its state for each of them. The state could no longer be just an int, but maybe a map { "key1": 0, "key2": 0 } etc, for all keys provided.

Then the component would need to be configured with which key to display.

Lastly, the reducer would need to iterate all the keys it was configured with, and increase the right key in the state.

Drawback is that this is potentially a breaking change since the state will change type Number -> Object. I have not decided if I consider the state part of the compatability though. The layout is not documented anywhere, so probably no - this change would be fine.

In regards to the multi-nprogress and it putting an id -> couldn't we use the template option to render a component of our choice?

mstruensee commented 5 years ago

i was thinking the same about an array ... let me give that a try and see if i like that better ...