Code Review! #7
Code Review! #7
Conversation
…avbar, add styles
…page when clicking on collection box
…ions and selector
Collections list page | add new collection
Minor fixes
Development
| reviewContent: reviewContent, | ||
| collectionId: collectionId, | ||
| bookId: bookId, | ||
| }; |
There was a problem hiding this comment.
no need to repeat the key/value in an object if you're going to call the keys the same as the constants are called. this works:
const newReviewdata = { reviewTitle, reviewContent, collectionId, bookId }
There was a problem hiding this comment.
oh, yes, the same in backend, thanks
|
|
||
| for (let index = 0; index < 5; index++) { | ||
| stars += index < rating ? "★" : "☆"; | ||
| } |
| // eslint-disable-next-line | ||
| book.categories?.forEach((item) => { | ||
| categories += item; | ||
| }); |
There was a problem hiding this comment.
const categories = book.categories.join("") i think works better. Same for authors
| {userCollections[0]?.user.name || user.name}'s collections | ||
| </h1> | ||
| <div className="collectionsContainer"> | ||
| {userCollections?.map((item) => ( |
There was a problem hiding this comment.
nice work using the new optional chaining stuff! it looks cool! only so you are aware, this feature isn't supported in every browser yet cause its pretty new, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining. Shouldn't be a problem unless its a really old browser
There was a problem hiding this comment.
ok, good to know that, I didn't suppose optional chaining isn't supported in every browser
|
|
||
| useEffect(() => { | ||
| dispatch(fetchFullData()); | ||
| }, [dispatch, setBooksCollections]); |
There was a problem hiding this comment.
in this case it shouldn't affect in any way, but careful on passing extra stuff to your useEffects because if its a variable you pass and it changes the useEffect will fire again
| booksCollections.map((item) => { | ||
| if (!item.review) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
idk if you know but this is called early-returns and its a pretty good practice, I really like it. If you dont have the stuff you need for a function, return. don't chain if and elses forever, 👍
src/store/bookPage/reducer.js
Outdated
|
|
||
| default: { | ||
| // do nothing | ||
| } |
There was a problem hiding this comment.
default => return state, always!
| @@ -0,0 +1,3 @@ | |||
| export const selectBookDetails = (state) => { | |||
| return state.bookPage; | |||
| }; | |||
There was a problem hiding this comment.
small stuff, but if you open an arrow function and immediately return, its better to write it in automatic return style
export const selectBookDetails = (state) => state.bookPage
src/store/collectionPage/reducer.js
Outdated
| } | ||
| } | ||
|
|
||
| return newState; |
There was a problem hiding this comment.
I like the creativity but i'd stick to the usual way of writing reducers (returning in every case) 😄
| } | ||
|
|
||
| return newState; | ||
| } |
There was a problem hiding this comment.
more thoughts on the reducers as i scroll down hahah.
For example, each time you dispatch an action, all reducers get called. Then one of them matches the action and returns something to change the state.
The way you have it set up, every time an action is dispatch every state changes, because you are building a new state object (with all the stuff the old one had yes, but for JS its still a new object). So every selector gets activated again, everything refreshes essentially.
There was a problem hiding this comment.
Ok, I think, I understand what you mean. I remember, Rein advised to write reducers in 'the usual way', but I started to do it differently from the very beginning (just because it was simpler to me). Of course I need to switch to the 'correct' way. Btw. I expected the comment about my reducers...
change reducers format
(setCollection)
Bumps [axios](https://github.com/axios/axios) from 0.19.2 to 0.21.1. - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v0.21.1/CHANGELOG.md) - [Commits](axios/axios@v0.19.2...v0.21.1) Signed-off-by: dependabot[bot] <support@github.com>
Bump axios from 0.19.2 to 0.21.1
No description provided.