Expose who has liked the parts #192 #199

Merged
yencolon merged 5 commits from main into main 2021-01-24 03:14:50 +01:00
yencolon commented 2021-01-20 04:53:41 +01:00 (Migrated from github.com)

192

@Irev-Dev check how it looks.

image

[192](https://github.com/Irev-Dev/cadhub/issues/192) @Irev-Dev check how it looks. ![image](https://user-images.githubusercontent.com/32972934/105124822-52cfe680-5ab1-11eb-83bd-8785303e59fa.png)
Irev-Dev commented 2021-01-20 10:27:55 +01:00 (Migrated from github.com)

Thanks @yencolon, This looks really good.
I've had quick look at the code and I'm impressed. Looks like I've definitely got Redwood figured out.
I'm a little tired atm, but I hope to have a proper look tomorrow and hopefully merge.

Thanks @yencolon, This looks really good. I've had quick look at the code and I'm impressed. Looks like I've definitely got Redwood figured out. I'm a little tired atm, but I hope to have a proper look tomorrow and hopefully merge.
Irev-Dev (Migrated from github.com) requested changes 2021-01-20 20:36:03 +01:00
Irev-Dev (Migrated from github.com) left a comment

I left a couple of comments for tiny changes.
One other thing I noticed is that if you look at the reactions then add a reaction, then look at the reactions again the new reaction doesn't appear. Here's a vid.

https://user-images.githubusercontent.com/29681384/105225513-cdccf600-5bb2-11eb-8fea-c49e0538fbe2.mov

Basically we need to tell apollo to update one of the queries.
Here's some documentation about it https://www.apollographql.com/blog/when-to-use-refetch-queries-in-apollo-client/
There's also an example of where I've used it in web/src/components/IdePartCell/IdePartCell.js

const [forkPart] = useMutation(FORK_PART_MUTATION, {
    refetchQueries: [
      {
        query: UsersPartsQuery,
        variables: { userName: user?.userName },
      },
    ],
    . . . . 

So I'm pretty sure you would want to put the same thing in web/src/components/PartCell/PartCell.js for TOGGLE_REACTION_MUTATION and reference the query you made in this PR.
Basically, its telling apollo that when a user adds a reaction with TOGGLE_REACTION_MUTATION than update this other query so that everything stays up-to-date?

Does that make sense?

I left a couple of comments for tiny changes. One other thing I noticed is that if you look at the reactions then add a reaction, then look at the reactions again the new reaction doesn't appear. Here's a vid. https://user-images.githubusercontent.com/29681384/105225513-cdccf600-5bb2-11eb-8fea-c49e0538fbe2.mov Basically we need to tell apollo to update one of the queries. Here's some documentation about it https://www.apollographql.com/blog/when-to-use-refetch-queries-in-apollo-client/ There's also an example of where I've used it in `web/src/components/IdePartCell/IdePartCell.js` ``` const [forkPart] = useMutation(FORK_PART_MUTATION, { refetchQueries: [ { query: UsersPartsQuery, variables: { userName: user?.userName }, }, ], . . . . ``` So I'm pretty sure you would want to put the same thing in `web/src/components/PartCell/PartCell.js` for `TOGGLE_REACTION_MUTATION` and reference the query you made in this PR. Basically, its telling apollo that when a user adds a reaction with `TOGGLE_REACTION_MUTATION` than update this other query so that everything stays up-to-date? Does that make sense?
Irev-Dev (Migrated from github.com) commented 2021-01-20 20:25:35 +01:00

I've rarely just straight black text in the app, so lets add className="text-gray-700" for consistency.

I've rarely just straight black text in the app, so lets add `className="text-gray-700"` for consistency.
@@ -0,0 +17,4 @@
export const Loading = () => <div>Loading...</div>
export const Empty = () => <div>Empty</div>
Irev-Dev (Migrated from github.com) commented 2021-01-20 20:22:32 +01:00

Let's make the empty state look a little nicer, even just

<div className="text-center py-8 font-roboto text-gray-700">
  No reactions to this part yet 😕
</div>

Should be good

Let's make the empty state look a little nicer, even just ``` <div className="text-center py-8 font-roboto text-gray-700"> No reactions to this part yet 😕 </div> ``` Should be good
yencolon commented 2021-01-20 21:44:59 +01:00 (Migrated from github.com)

Oh, understandable. Thanks for the corrections. I will fix it :)

Oh, understandable. Thanks for the corrections. I will fix it :)
Irev-Dev commented 2021-01-24 03:14:44 +01:00 (Migrated from github.com)

@yencolon I appreciate you putting up this PR. Since it's been a few days I thought maybe you ran out of steam, which is fair enough, so I've got another PR to make the few weeks I suggested.

You can see the changes here. https://github.com/Irev-Dev/cadhub/pull/201

Thanks again :)

@yencolon I appreciate you putting up this PR. Since it's been a few days I thought maybe you ran out of steam, which is fair enough, so I've got another PR to make the few weeks I suggested. You can see the changes here. https://github.com/Irev-Dev/cadhub/pull/201 Thanks again :)
yencolon commented 2021-01-24 14:46:51 +01:00 (Migrated from github.com)

@Irev-Dev Oh, sorry, I was expecting to do this on Sunday (today 😅), I was busy with work. But, it's ok, thanks for finishing it up!

@Irev-Dev Oh, sorry, I was expecting to do this on Sunday (today 😅), I was busy with work. But, it's ok, thanks for finishing it up!
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: h3n3/cadhub#199