RE: [KnackSteem] - Edit/Reply for Articles and Comments

You are viewing a single comment's thread from:

RE: [KnackSteem] - Edit/Reply for Articles and Comments

View the full context

gregory.latinier (65)in #utopian-io • 6 years ago

Hi there, this is great work! You've detailed your work that's good.

Let's go for the review:

Starting with the useless comment war :D
https://github.com/ateufel/knacksteem.org/blob/b1ebe15af3dfb845204619b4c72e765b32c50df4/src/components/Editor/index.js#L195 => You should add the second argument to mapActions instead of using https://github.com/ateufel/knacksteem.org/blob/b1ebe15af3dfb845204619b4c72e765b32c50df4/src/components/Editor/index.js#L97
https://github.com/ateufel/knacksteem.org/blob/b1ebe15af3dfb845204619b4c72e765b32c50df4/src/components/Editor/index.js#L97 => You shouldn't await for the dispatch but have your action send a ACTION_SUCCESS when it's done so that the reducer handles it.
Your use of redux and redux action is a bit weird. You should find some documentation / articles.
I sort of expected to see something like this

Aside from the use of redux that I find a bit weird your code is great! Good job on this pull request!

Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.

To view those questions and the relevant answers related to your post, click here.

Need help? Write a ticket on https://support.utopian.io/.
Chat with us on Discord.
[utopian-moderator]

6 years ago in #utopian-io by gregory.latinier (65)

Sort:

luschn (63) 6 years ago

Thanx, that is great feedback! About redux action binding: I am actually using your suggestion in another project (with Next.js), so I know exactly what you mean and you are absolutely right. I also like the way how you avoid a separate mapStateToProps function by just putting it in the connect param in your screenshot. Still readable, but more compact.
About awaiting the dispatch: There´s some routing and other stuff involved, so while I agree that actions should handle success responses, in this case it´s a lot easier to just use async/await to wait for the dispatch to return. Although, put that into an action that does the routing or other stuff...will consider that for a refactoring session :)
Again, thanx for the valuable and detailed feedback!