✨ DOP-4662 adds community pill to cards for when using tags

[caesarbell]

Stories/Links:

DOP-4662

Current Behavior:

Python Landing

Docs Landing

Staging Links:

Staged: Python Landing

Staged: Docs Landing (built from a forked repo)

Notes:

• For this PR, I modified the community pill component to be reusable and swapped out the Tag component to use the new community pill component.

README updates

• • This PR introduces changes that should be reflected in the README, and I have made those updates.
• • This PR does not introduce changes that should be reflected in the README

[mmeigs]

Does variant need to default to "lightgray"?

[caesarbell]

@mmeigs good question, in this case, yes the variant prop for Badge should default to lightgray to support the original use case of the community pill.

I am doing so within the Badge component i.e.

<Badge variant={communityPillVariants[variant] || Variant.LightGray}>{text}</Badge>

[caesarbell]

I could re-write this to be more like

const CommunityPillLink = ({ nodeData, variant = 'lightgray', text = 'community built' }) => {
  const { argument, options: { url } = {} } = nodeData || {};

  return (
    <div>
      {nodeData && argument && url && <Link to={url}>{getPlaintext(argument)}</Link>}
      <Badge variant={communityPillVariants[variant]}>{text}</Badge>
    </div>
  );
};

if you prefer.

[mayaraman19]

i like this!

[caesarbell]

@mayaraman19 the former or latter?

[mmeigs]

Wow, my brain. Yep - you're so right. I guess it would be nice to have the defaults be declared the same way! Either way is fine with me.

[caesarbell]

@mmeigs no worries, it's the middle of the week and our brains have been on overdrive. I will update the code to declare it the same way.

[mayaraman19]

this is a little bit difficult to understand because of the repeated references to param0, nodeData, etc. Might be better to remove + ensure the code is straightforward to understand in order to reduce confusion?

[caesarbell]

@mayaraman19 I can remove this, it was to help when hovering your mouse over the component when called in another file.

[mayaraman19]

i see! that is a nice touch. I guess the confusion lies in naming param0 since it's not mentioned by name in the code, as well as I don't understand why some have brackets [] around them, since they don't seem to be array values? minor & non-blocking though, i trust your judgement!

[caesarbell]

@mayaraman19 I went with your feedback, no need to add confusion when it's not needed. The brackets indicated the default value.

[mayaraman19]

is there a way to define these as optional using the nodeData prop instead i.e.

const CommunityPillLink = ({ nodeData: {argument?, options?: {url}}, variant, text = 'community built' }) => {
...

or something like that. I'm forgetting my Javascript currently though so idk if this is possible

[caesarbell]

@mayaraman19 just out of curiosity, what are you trying to achieve, readability? I am asking so I can better understand the suggestion/comment.

[caesarbell]

@mayaraman19 also, from what I understand that might not be doable in vanilla JavaScript. That is more of a Typescript approach.

[mayaraman19]

i guess the intent was to use the convention we typically use for getting values from nodeData as well as remove the need to check for nodeData here. but if it is not possible that's okay, it's very minor

[mayaraman19]

i see! that is a nice touch. I guess the confusion lies in naming param0 since it's not mentioned by name in the code, as well as I don't understand why some have brackets [] around them, since they don't seem to be array values? minor & non-blocking though, i trust your judgement!