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

feat(comp): remove useStyles from Select #1484

Merged
merged 2 commits into from
Nov 29, 2021
Merged

feat(comp): remove useStyles from Select #1484

merged 2 commits into from
Nov 29, 2021

Conversation

ti10le
Copy link
Contributor

@ti10le ti10le commented Nov 29, 2021

@changeset-bot
Copy link

changeset-bot bot commented Nov 29, 2021

🦋 Changeset detected

Latest commit: bfbb00c

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

This PR includes changesets to release 6 packages
Name Type
@marigold/components Patch
docs Patch
@marigold/icons Patch
@marigold/theme-b2b Patch
@marigold/theme-core Patch
@marigold/theme-unicorn 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

@github-actions github-actions bot added the type:feature New feature or component label Nov 29, 2021
@coveralls
Copy link

coveralls commented Nov 29, 2021

Coverage Status

Coverage decreased (-0.3%) to 99.675% when pulling bfbb00c on useStyles-Select into b655a77 on main.

@@ -94,7 +83,7 @@ export const Select = ({
{required ? (
<Box as="span" display="inline-flex" alignItems="center">
{props.label}
<Required size={16} className={errorClassName} />
<Box as={Required} size={16} css={{ color: 'error' }} />
Copy link
Member

Choose a reason for hiding this comment

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

lassen das so, aber eigentlich haben wir ja "currentColor" sprich wenn das label die farbe ändert, ändert sich das Icon auch mit! :)

<Box
as={ArrowUp}
size={16}
css={{ fill: disabled ? 'disabled' : 'text' }}
Copy link
Member

Choose a reason for hiding this comment

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

Was genau macht das? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

icon color

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We dont need this because if disabled then ArrowUp is never visible. Uuups

@sebald sebald merged commit 24367e6 into main Nov 29, 2021
@sebald sebald deleted the useStyles-Select branch November 29, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants