Fixed reaction count on comments #4243
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kurtisassad
requested review from
jnaviask,
zakhap,
dillchen and
CowMuon
as code owners
|
@kurtisassad this is actually an opportunity for react-query rather than zustand, given that the state originates in an API call + is updated via API call mutations. Zustand is best used with state that is carried across many different component hierarchies (avoids massive prop drilling), while react-query works best when there's a single source of truth from the API that gets modified by user interaction. We can merge this PR as-is, but I will create another ticket for react-query usage. |
kurtisassad
force-pushed
the
ka.reaction-count-fix
branch
from
9cb1ecd to
da95eeb
Compare
|
Teed up #4244 to be a separate PR. |
jnaviask
requested changes
Jun 14, 2023
packages/commonwealth/client/scripts/views/components/ReactionButton/CommentReactionButton.tsx
Show resolved
Hide resolved
packages/commonwealth/client/scripts/views/pages/view_thread/ViewThreadPage.tsx
Outdated
Show resolved
Hide resolved
jnaviask
approved these changes
Jun 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to Issue
Closes: #4186
Description of Changes
Test Plan
Other Considerations
I guess this would be a good candidate for using Zustand. I know Marcin is out, so I am not sure if we want to go this route (since it might be better if he could review it to make sure we are all on board with the structure). Anyways if we want to still go ahead and use Zustand, let me know in the comments, and I will convert this PR.