Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: record object chip background color when idle (not hovered) #4662

Conversation

emadbaqeri
Copy link
Contributor

@emadbaqeri emadbaqeri commented Mar 26, 2024

Fixes #4651

Screen.Recording.2024-03-26.at.15.08.16.mov

Copy link

github-actions bot commented Mar 26, 2024

Welcome!

Hello there, congrats on your first PR! We're excited to have you contributing to this project.
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

Generated by 🚫 dangerJS against ecbb695

@FelixMalfait
Copy link
Member

Thanks @emadbaqeri

I think the issue only referred to StyledBoardCardHeader only (check RecordBoardCard.tsx and probably change it there). You'll have to add this variant to the recordChip as I see it's only available at the entityChip level now

cc @Bonapara

@emadbaqeri
Copy link
Contributor Author

emadbaqeri commented Mar 26, 2024

StyledBoardCardHeader

I not sure I understand this so I'm asking a dummy question, you mean I should do something like this?

export type RecordChipProps = {
  ...,
  variant?: EntityChipVariant;
};


<RecordChip variant={EntityChipVariant.Transparent} />

And this way we can control the RecordChip variant property from outside and this should be better I think as you mentioned.

@FelixMalfait
Copy link
Member

Yes that's how I would have done it too! That way we can follow the design that's on the issue

@emadbaqeri
Copy link
Contributor Author

Yes that's how I would have done it too! That way we can follow the design that's on the issue

I've updated the RecordChip component to get a variant prop as you said, now ready to merge :)

@bosiraphael bosiraphael self-assigned this Mar 27, 2024
@bosiraphael bosiraphael self-requested a review March 27, 2024 13:24
Copy link
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for contributing @emadbaqeri :)

Copy link
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should pass the variant in StyledBoardCardHeader, not change the default variant for all RecordChips :)
(look at the design on the issue, it's only the header of the card)

@ijreilly
Copy link
Contributor

@FelixMalfait is it ok for you now?

@ijreilly ijreilly dismissed FelixMalfait’s stale review April 15, 2024 13:40

changes implemented

@ijreilly ijreilly merged commit 42e50cb into twentyhq:main Apr 15, 2024
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Card Name Chip Design on Kanban Views
4 participants