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

multiple redux spinners #24

Open
mstruensee opened this issue Apr 13, 2018 · 18 comments
Open

multiple redux spinners #24

mstruensee opened this issue Apr 13, 2018 · 18 comments

Comments

@mstruensee
Copy link

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?

@mstruensee mstruensee changed the title curry spinner multiple redux spinners Apr 13, 2018
@noseglid
Copy link
Member

noseglid commented Sep 7, 2018

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
Copy link

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

@mstruensee
Copy link
Author

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
Copy link

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
Copy link
Author

mstruensee commented Sep 13, 2018

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
Copy link
Member

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

  • The Spinner component must have a configurable path in the state. It can't look directly at pendingTasks but that must be passed into it as a property.

  • The reducer must be able to reduce into different paths of the state.

  • The actions must contain information about which part of the state to modify.

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
Copy link
Author

mstruensee commented Sep 17, 2018

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' [ x ]: begin });

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
Copy link
Author

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

@noseglid
Copy link
Member

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
Copy link
Author

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

@mstruensee
Copy link
Author

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

@mstruensee
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

mstruensee commented Sep 21, 2018

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

store.dispatch({ type: "FETCHING_ASYNC_DATA", [pendingTask]: begin, [anotherTask]: begin }) store.dispatch({ type: "FETCHING_ASYNC_DATA", [pendingTask]: begin, [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('body)]: begin, [pendingTask('container1')]: begin }) store.dispatch({ type: "FETCHING_ASYNC_DATA", [pendingTask('body)]: begin, [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
Copy link
Author

what are your thoughts?

@noseglid
Copy link
Member

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
Copy link
Author

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

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