Skip to content

Adds disabled checkbox text color #97

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

Merged

Conversation

lloydaf
Copy link
Contributor

@lloydaf lloydaf commented May 27, 2021

What:
This PR fixes the color of the text of the disabled state for the Checbox component.

Why:
This is in regard to SUP-11111

How:

  • Pass the disabled property to the Text component inside Checkbox

    Media:
    Design

Screenshot of documentation

Note: This PR should ideally be merged only after the preceding one is merged, so that we can isolate changes in the merge commit with the ticket (currently, changes for SUP-11112 are included here as well)

@duranmla
Copy link
Contributor

Now I understand #96 nonetheless, I am not sure if we should achieve this by using the parent (although I understand it is not a "recommended" practice on styled-components).

Kinda,

Checkbox = css`
    ${Text} {
        // disable styles
    }
`

But at the same time Why not allow disabled prop on Text. I am a bit hesitant right now, I would like to read some other folks comments

@lloydaf
Copy link
Contributor Author

lloydaf commented May 27, 2021

In my opinion, since we're adding a disabled property to Text, this is the best way to ensure consistency. Tomorrow, if we decide to change the disabled colors for example, we only have to change it in Text and it will be reflected everywhere else. So in my opinion this is also easier from a maintainability perspective, but I am open to hearing different opinions as well.

@nlopin nlopin changed the title Sup 11111/disabled checkbox text color Adds disabled checkbox text color May 28, 2021
@duranmla duranmla merged commit d2dc31d into freenowtech:main Jun 3, 2021
github-actions bot pushed a commit that referenced this pull request Jun 3, 2021
## [1.2.0](v1.1.0...v1.2.0) (2021-06-03)

### Features

* **text:** adding the disabled text color, fixing inverted text colors ([#96](#96)) ([40b529d](40b529d))

### Bug Fixes

* **checkbox:** Adds disabled checkbox text color ([#97](#97)) ([d2dc31d](d2dc31d))
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2021

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lloydaf lloydaf deleted the SUP-11111/disabled-checkbox-text-color branch June 3, 2021 15:18
@duranmla
Copy link
Contributor

duranmla commented Jun 4, 2021

@all-contributors please add @lloydaf for code collaboration after this pull request

@allcontributors
Copy link
Contributor

@duranmla

I've put up a pull request to add @lloydaf! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants