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

Added color prop to the Tag component #2338

Closed
wants to merge 9 commits into from

Conversation

AbhayVAshokan
Copy link
Member

Description

  • Added: color prop to the Tag component.

Checklist

  • I have made corresponding changes to the documentation.
  • I have updated the types definition of modified exports.
  • I have verified the functionality in some of the neeto web-apps.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added proper data-cy and data-testid attributes.
  • I have added the necessary label (patch/minor/major - If package publish
    is required).

Reviewers

@AbhayVAshokan _a
patch _t

@neetogit-bot neetogit-bot bot added the patch Releases small requests or bug fixes. label Oct 10, 2024
@neetodeploy neetodeploy bot temporarily deployed to neeto-ui-kpyu-pr-2338 October 10, 2024 05:46 Inactive
@neetodeploy neetodeploy bot temporarily deployed to neeto-ui-kpyu-pr-2338 October 10, 2024 06:02 Inactive
@AbhayVAshokan
Copy link
Member Author

@Tsudhishnair _a please use this URL to review. I have built a custom component to help with the PR review.

Here, I have used all the colors provided by the GitHub labels color palette by default. I have added a color picker to test it with different colors.

Light mode

image

Dark mode

image

  • Here, all the colors look good in the dark mode. The lighter colors do not look good in the light mode.
  • GitHub uses a different set of colors for light and dark mode. Hence, the labels look good in all the cases.
  • It can be observed that all the colors look good for "solid" type in both the themes.

@Tsudhishnair please have a look at the UI and LMK:

  • If any further changes need to be taken up for the UI (different calculation).
  • The list of colors that would be the best fit for neeto-tags-nano.
  • Any other suggestions.

@neetogit-bot neetogit-bot bot assigned Tsudhishnair and unassigned AbhayVAshokan Oct 10, 2024
@praveen-murali-ind
Copy link
Contributor

@praveen-murali-ind _A Self-assigning to work on the colors.

Copy link
Contributor

@praveen-murali-ind praveen-murali-ind left a comment

Choose a reason for hiding this comment

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

@AbhayVAshokan _a, as discussed on the call, let’s remove the new color prop and use the existing style prop instead.

cc @Tsudhishnair

@AbhayVAshokan
Copy link
Member Author

As per the discussion, I am closing this PR. We will add new styles as part of the Tag component and the colors will be added as new styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Releases small requests or bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add color prop to Tag component
3 participants