Discussion Count Redux #770
Conversation
| useEffect(() => { | ||
| fetchDiscussionThreads(false, true); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| fetchDiscussionThreads(false, true); | ||
| }, [filter, showTwitterComments]); |
There was a problem hiding this comment.
Could you double check logic here. Seems unnecessary to double fetch like this
| @@ -380,7 +386,7 @@ const DiscussionTab = (props) => { | |||
| ) : showTwitterComments ? ( | |||
| calculateCount() | |||
| ) : ( | |||
| props.calculatedCount | |||
| <DiscussionCount /> | |||
There was a problem hiding this comment.
Hmm, not sure if this needs to be a component. If we already have an access to the count via redux, we could probably just display it here.
If this was necessary, could you explain?
There was a problem hiding this comment.
Hey, I made it like this because I thought that was being asked: "Create a "root component" for Discussions that will fetch & propagate the redux data rather than having the fetch being done at the page-level component".
I may have misinterpreted what you were saying.
There was a problem hiding this comment.
I see your point though.
In the branch discussion-redux, I made this component also take care of the loading state so it may gives it (being a component) a bit more validity. Let me know what you want me to do
| @@ -467,7 +473,7 @@ const DiscussionTab = (props) => { | |||
| type="beat" | |||
| /> | |||
| ) : ( | |||
| props.calculatedCount | |||
| <DiscussionCount /> | |||
Features
Resolves 2 / 3 of #759