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: make CheckBox styling respect the figma design #1806

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rami-pro
Copy link

@rami-pro rami-pro commented Nov 4, 2024

What does it do?

I just added styling to my checkbox styeled component, and changed icon's color based on the state of the checkbox if it's disabled or not

Why is it needed?

refer issue : strapi/strapi#22032

How to test it?

Check on the storybook and compare to figma design at link

Related issue(s)/PR(s)

strapi/strapi#22032

Copy link

changeset-bot bot commented Nov 4, 2024

🦋 Changeset detected

Latest commit: 9d1e69a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@strapi/design-system Patch
@strapi/icons Patch
@strapi/ui-primitives Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Nov 4, 2024

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

Name Status Preview Comments Updated (UTC)
design-system ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2024 4:01pm

@hanpaine
Copy link
Contributor

hanpaine commented Nov 4, 2024

Hey @rami-pro nice catch 🚀 we'll take a look as soon as possible 🙏

@rami-pro
Copy link
Author

rami-pro commented Nov 14, 2024

@hanpaine is there anything to change to merge this PR ?

@hanpaine
Copy link
Contributor

Hey @rami-pro it hasn't been reviewed by the dev team yet; they'll let you know when they can get to it 🙏

@markkaylor markkaylor self-requested a review November 26, 2024 12:12
@markkaylor markkaylor self-assigned this Nov 26, 2024
Copy link
Contributor

@markkaylor markkaylor left a comment

Choose a reason for hiding this comment

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

@rami-pro You will need to add a changeset to your PR. See contributing docs for more information. Let me know if anything is not clear.

The cursor is still setting the cursor to 'pointer' on hover when the checkbox is disabled. @lucasboilly is that what we want?

@markkaylor markkaylor added pr: fix This PR is fixing a bug source: design-system relates to design-system package labels Nov 26, 2024
@lucasboilly
Copy link

Hey @markkaylor :)

No, we should change the cursor to its disabled version. It's supposed to look like this:

CleanShot 2024-11-26 at 14 02 51@2x

@rami-pro
Copy link
Author

Do we have the disabled version of the cursor on strapi/icons, didn't find one ? @lucasboilly @markkaylor

@lucasboilly
Copy link

@rami-pro it isn't in the icons, I think it's supposed to be fixed by typing something like cursor: not-allowed. Am I right @markkaylor?

@markkaylor
Copy link
Contributor

@rami-pro it isn't in the icons, I think it's supposed to be fixed by typing something like cursor: not-allowed. Am I right @markkaylor?

Yes that's right you should be able to apply the cursor styles with css

https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

@rami-pro rami-pro force-pushed the fix/checkbox-styling branch from fedceeb to 9d1e69a Compare December 20, 2024 15:56
@rami-pro
Copy link
Author

I answwered your requests, thanks @lucasboilly @markkaylor for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community pr: fix This PR is fixing a bug source: design-system relates to design-system package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Design] Disabled checkboxes in Super Admin settings have a blue border and the check hasn’t the right color
4 participants