Discussions Card Component Design Only #2339

HS-90 · 2022-09-23T18:02:53Z

Description: relates to #2252

This PR creates the initial design for the two different types of discussion cards as shown in this design:
https://www.figma.com/file/f9xRyl1bJOjtBRd8zFORzg/c0d3

The submitted PR is not designed 1 to 1 compared to figma, but we can discuss and come up with a finalized design if necessary.
You can view this design by running yarn storybook and view DiscussionsCard. Additionally, here are screenshots of the design below:

Main Card:

Sub Card:

vercel · 2022-09-23T18:02:58Z

@HS-90 is attempting to deploy a commit to the c0d3-prod Team on Vercel.

A member of the Team first needs to authorize it.

codecov · 2022-09-23T18:11:50Z

Codecov Report

Merging #2339 (3d33afb) into master (012c830) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 3d33afb differs from pull request most recent head 3b93981. Consider uploading reports for the commit 3b93981 to get more accurate results

@@            Coverage Diff            @@
##            master     #2339   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          181       182    +1     
  Lines         3187      3192    +5     
  Branches       843       845    +2     
=========================================
+ Hits          3187      3192    +5

Impacted Files	Coverage Δ
components/DiscussionsCard/DiscussionsCard.tsx	`100.00% <100.00%> (ø)`

vercel · 2022-09-23T18:14:37Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
c0d3-app	✅ Ready (Inspect)	Visit Preview	Sep 28, 2022 at 1:31AM (UTC)

bryanjenningz · 2022-09-23T22:24:56Z

components/DiscussionsCard/DiscussionsCard.tsx

+          <span> - </span>
+          <span>{timeStamp}</span>
+        </div>
+        <div className={styles.expand_button} onClick={expandClick()}>


I think you meant to put onClick={expandClick} instead.

bryanjenningz · 2022-09-23T22:25:23Z

components/DiscussionsCard/DiscussionsCard.tsx

+          </div>
+        </div>
+        <div className={styles.likes_container}>
+          <div className={styles.likes_button} onClick={likeClick()}>


Change to onClick={likeClick}.

bryanjenningz · 2022-09-23T22:25:42Z

components/DiscussionsCard/DiscussionsCard.tsx

+            <ThumbsupIcon className={styles.thumbs_up_down} />{' '}
+            <span className={styles.number_shown}>{likes}</span>
+          </div>
+          <div className={styles.likes_button} onClick={dislikeClick()}>


Change to onClick={dislikeClick}.

bryanjenningz · 2022-09-23T22:27:41Z

components/DiscussionsCard/DiscussionsCard.tsx

+} from '@primer/octicons-react'
+
+export type DiscussionsCardProps = {
+  discussionType: 'main' | string


You can change this to type 'main' | 'sub' if that makes more sense.

@HS-90 Is there any potential that there will be more than 2 possible values for discussionType? Also, what is the reason for different styling options? Do we have different styling on different pages for DiscussionCard component?

I'm thinking that if we know there will never be more than 2 possible values for discussionType, then we might as well simplify things make the prop a boolean value, ex. subMode?: boolean

If theres high potential that discussionType will have more than 2 possible values, id suggest we use a mapping approach instead of using ternary operators(see lines 39 and 42 for example)

Example of mapping approach:

className={styles[`card_body_${discussionType}]}

The above might be a bad example.. im not sure its a good idea for name of theclass to be part of the logic in this kind of way because it makes the logic more complex

A more simple approach would probably be to have main and sub as keys in a hashmap, and then have as its values the corresponding styles for them

If theres high potential that discussionType will have more than 2 possible values, id suggest we use a mapping approach instead of using ternary operators(see lines 39 and 42 for example)

There should be only 2 types if we are going by design, one is a main card, which will be the original comment or thread, then replies will be the sub design, branching off of the original comment.

A subcard can also have subcards branching off of them, kind of like how reddit works. but the design should be the same for every subcard, if that makes sense. I think I agree with your boolean approach.

If the only purpose of having a boolean is to check if a reply is the main one, wouldn't be better to instead check if the subreply doesn't have any parent comment and style accordingly?
Also, should we consider that any subreply that has n replies would be considered a main? if so, wouldn't that make things a bit confusing?
We didn't get to this point in the design meetings.

If the only purpose of having a boolean is to check if a reply is the main one, wouldn't be better to instead check if the subreply doesn't have any parent comment and style accordingly?

Why is it better?

After a bit of though, it seems like it won't make any difference in our case, the only case it would be useful is if we want to style subthread replies with different levels/shades of colors.

SlyBouhafs · 2022-09-24T19:01:57Z

Preview URL: https://deploy-preview-2339--youthful-thompson-448332.netlify.app/?path=/story/components-discussionscard--discussions-card-main

SlyBouhafs · 2022-09-24T19:15:34Z

components/DiscussionsCard/DiscussionsCard.tsx

+          <span>{timeStamp}</span>
+        </div>
+        <div className={styles.expand_button} onClick={expandClick()}>
+          ...


What does the expandClick function do in this case? I remember from the design sessions the idea behind was to use it to edit/delete the comment, but it looks like those two have their own button in this design, should we keep it?

It depends. Do we want to show every response to the thread(including responses to responses) upon load, or do we want the user to click first before it expands?

Interesting, i didn't think it would be used for collapsing comments.
I don't think hiding the thread upon load is a good idea, threads should all be visible and if the thread is too long, we can add a button to show more responses.
Maybe we can make the comments with a negative score collapsed by default? That would be a good use for it. Do we have a design on the collapsed state?

Do we have a design on the collapsed state?

Only the discussion cards so far. Haven't implemented the full UI yet. I agree that we can load them all to a certain point.
Should we have all responses feed to main parent post(the purple design), even if they were responding to a sub post? Or should we have it like reddit, where it continues branching out if you reply to a sub post?

Should we have all responses feed to main parent post(the purple design), even if they were responding to a sub post? Or should we have it like reddit, where it continues branching out if you reply to a sub post?

I think having something similar to reddit is the way to go, we just need to make sure that the design will not break once we have too many branches.

SlyBouhafs · 2022-09-24T19:19:14Z

components/DiscussionsCard/DiscussionsCard.tsx

+        </div>
+        <div className={styles.likes_container}>
+          <div className={styles.likes_button} onClick={likeClick()}>
+            <ThumbsupIcon className={styles.thumbs_up_down} />{' '}


If i remember correctly, during the design meetings we agreed on not having a like/dislike button and instead use arrows and only and show a sum of the votes.

In the designs we have two versions and i think we agreed on this one:

We also discussed another idea where we should only have the like button:

@flacial Can you confirm which one did we agree on?

I think without likes. On v2 of the discussions component, we can implement likes then.

I think without likes. On v2 of the discussions component, we can implement likes then.

Should we not include likes in the current PR? And add the up down arrows in a future PR?

Or are you talking about V2 as in, after this initial discussions component has been fully implemented without likes.

Or are you talking about V2 as in, after this initial discussions component has been fully implemented without likes.

Yes, after this initial design.

flacial · 2022-09-24T20:34:37Z

Some comments:

… removed css for deleted parts, updated DiscussionCardProps

HS-90 · 2022-09-27T21:10:12Z

Updated screenshots of designs:

Link to storybook: https://deploy-preview-2339--youthful-thompson-448332.netlify.app/?path=/story/components-discussionscard--discussions-card-main

If everyone is ok with the initial designs of these cards, we can merge and then update functionality in future PRs.

anthonykhoa · 2022-09-27T22:20:38Z

Updated screenshots of designs:

If everyone is ok with the initial designs of these cards, we can merge and then update functionality in future PRs.

The image used for reply looks like something id see on a go back button. However, i see you are using the reply icon from react. Any thoughts @bryanjenningz @HS-90 @SlyBouhafs @flacial ? IMO id rather see something like what reddit uses, they use something that looks like the outline of a txt message. I think the image they have next to the reply button is more closely associated with the idea of replying than compared to an image of an arrow pointing to the left

This isn't a blocker though, we can always update this later since this PR only adds the component. I just wanted to point this out

HS-90 · 2022-09-28T00:17:17Z

The image used for reply looks like something id see on a go back button. However, i see you are using the reply icon from react. Any thoughts @bryanjenningz @HS-90 @SlyBouhafs @flacial ? IMO id rather see something like what reddit uses, they use something that looks like the outline of a txt message. I think the image they have next to the reply button is more closely associated with the idea of replying than compared to an image of an arrow pointing to the left

I good with switching the icon. Is that also a react icon? Or a different library?

anthonykhoa · 2022-09-28T00:18:34Z

The image used for reply looks like something id see on a go back button. However, i see you are using the reply icon from react. Any thoughts @bryanjenningz @HS-90 @SlyBouhafs @flacial ? IMO id rather see something like what reddit uses, they use something that looks like the outline of a txt message. I think the image they have next to the reply button is more closely associated with the idea of replying than compared to an image of an arrow pointing to the left

I good with switching the icon. Is that also a react icon? Or a different library?

Im not sure about where to find an icon like reddit's to use.
This isn't a blocker though, we can always update this later since this PR only adds the component. I just wanted to point this out

SlyBouhafs · 2022-09-28T00:26:47Z

I good with switching the icon. Is that also a react icon? Or a different library?

@HS-90 Yeah, a similar icon is available in Octicons: https://primer.style/octicons/comment-16

… exerciseList

HS-90 · 2022-09-28T00:59:19Z

https://deploy-preview-2339--youthful-thompson-448332.netlify.app/?path=/story/components-discussionscard--discussions-card-sub

What it looks like with comment icon.

SlyBouhafs · 2022-09-28T01:05:12Z

https://deploy-preview-2339--youthful-thompson-448332.netlify.app/?path=/story/components-discussionscard--discussions-card-sub

What it looks like with comment icon.

Looks good! One small detail, i think 10px is too much for the border radius on the hover state, 5px would look a lot better!

HS-90 · 2022-09-28T01:53:41Z

https://deploy-preview-2339--youthful-thompson-448332.netlify.app/?path=/story/components-discussionscard--discussions-card-main

discussions card component design

dafb78c

HS-90 added the enhancement New feature or request label Sep 23, 2022

vercel bot deployed to Preview September 23, 2022 18:16 View deployment

bryanjenningz reviewed Sep 23, 2022

View reviewed changes

SlyBouhafs reviewed Sep 24, 2022

View reviewed changes

HS-90 and others added 2 commits September 24, 2022 18:19

Merge branch 'master' into exerciseList

a76694c

Merge branch 'master' into exerciseList

75b9dbf

vercel bot deployed to Preview September 26, 2022 14:23 View deployment

Merge branch 'master' into exerciseList

6b197a0

vercel bot deployed to Preview September 26, 2022 16:50 View deployment

Merge branch 'master' into exerciseList

530843f

vercel bot deployed to Preview September 27, 2022 00:48 View deployment

HS-90 and others added 3 commits September 26, 2022 20:48

Merge branch 'master' into exerciseList

027d006

updated after comments: removed likes, added padding to reply button,…

147763e

… removed css for deleted parts, updated DiscussionCardProps

Merge branch 'garageScript:master' into exerciseList

02d5c8b

vercel bot deployed to Preview September 27, 2022 21:16 View deployment

SlyBouhafs approved these changes Sep 27, 2022

View reviewed changes

anthonykhoa approved these changes Sep 28, 2022

View reviewed changes

HS-90 added 3 commits September 27, 2022 20:44

change from reply icon to comment icon

8b358d1

update storyshots

8164a24

Merge branch 'exerciseList' of https://github.com/HS-90/c0d3-app into…

8c2811f

… exerciseList

vercel bot deployed to Preview September 28, 2022 00:54 View deployment

SlyBouhafs and others added 2 commits September 28, 2022 02:09

Merge branch 'master' into exerciseList

da2c7a0

change button border-radius

3b93981

vercel bot deployed to Preview September 28, 2022 01:31 View deployment

HS-90 merged commit 212fa00 into garageScript:master Sep 28, 2022

Discussions Card Component Design Only #2339

Discussions Card Component Design Only #2339

Conversation

HS-90 commented Sep 23, 2022 • edited Loading

vercel bot commented Sep 23, 2022

codecov bot commented Sep 23, 2022 • edited Loading

Codecov Report

vercel bot commented Sep 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonykhoa Sep 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonykhoa Sep 23, 2022 • edited Loading

Choose a reason for hiding this comment

HS-90 Sep 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SlyBouhafs commented Sep 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SlyBouhafs Sep 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SlyBouhafs Sep 24, 2022 • edited Loading

Choose a reason for hiding this comment

SlyBouhafs Sep 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flacial commented Sep 24, 2022

HS-90 commented Sep 27, 2022 • edited Loading

anthonykhoa commented Sep 27, 2022 • edited Loading

HS-90 commented Sep 28, 2022

anthonykhoa commented Sep 28, 2022 • edited Loading

SlyBouhafs commented Sep 28, 2022 • edited Loading

HS-90 commented Sep 28, 2022

SlyBouhafs commented Sep 28, 2022

HS-90 commented Sep 28, 2022

HS-90 commented Sep 23, 2022 •

edited

Loading

codecov bot commented Sep 23, 2022 •

edited

Loading

vercel bot commented Sep 23, 2022 •

edited

Loading

anthonykhoa Sep 23, 2022 •

edited

Loading

anthonykhoa Sep 23, 2022 •

edited

Loading

HS-90 Sep 24, 2022 •

edited

Loading

SlyBouhafs commented Sep 24, 2022 •

edited

Loading

SlyBouhafs Sep 24, 2022 •

edited

Loading

SlyBouhafs Sep 24, 2022 •

edited

Loading

SlyBouhafs Sep 24, 2022 •

edited

Loading

HS-90 commented Sep 27, 2022 •

edited

Loading

anthonykhoa commented Sep 27, 2022 •

edited

Loading

anthonykhoa commented Sep 28, 2022 •

edited

Loading

SlyBouhafs commented Sep 28, 2022 •

edited

Loading