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(vara-ui): upd inputs #1694

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(vara-ui): upd inputs #1694

wants to merge 1 commit into from

Conversation

ereburg
Copy link
Collaborator

@ereburg ereburg commented Dec 4, 2024

No description provided.

border-radius: 4px;
background-color: #ffffff;
Copy link
Member

Choose a reason for hiding this comment

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

We can't put white background color as we can't guarantee that every application's background will be white too. It should be transparent.

@@ -63,6 +64,7 @@ const Input = forwardRef<HTMLInputElement, Props>(
className={cx(styles.input, styles[size], error && styles.error)}
placeholder={placeholder}
ref={ref}
disabled={disabled}
Copy link
Member

Choose a reason for hiding this comment

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

disabled is applied from attrs destructuring already.

@nikitayutanov
Copy link
Member

Another comments from previous PRs about UX and our style practices applies there too.

@nikitayutanov
Copy link
Member

Also, regarding general approach to dark mode styles.

What do you think about creating mixin, that will abstract dark mode for a certain element?
I'm not sure that it will work, but something like:

@mixin darkMode ($width) {
  :global(.dark-mode) & {
    @content;
  }
}

to utilize it like that:

.element {
  color: #fff;

  @include darkMode {
    color: #000;
  }
}

If it's working as expected, I believe it would be far easier to control color variations in the scope of a certain element.
Input styles are bright example of it, since some properties can be easilly forgotten or missed.

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

Successfully merging this pull request may close these issues.

3 participants