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

Add inputClassName to InputGroup props #5860

Closed
viveleroi opened this issue Jan 21, 2023 · 11 comments · Fixed by #5868
Closed

Add inputClassName to InputGroup props #5860

viveleroi opened this issue Jan 21, 2023 · 11 comments · Fixed by #5868

Comments

@viveleroi
Copy link

viveleroi commented Jan 21, 2023

I need a way to add a specific class name to the input inside of an InputGroup but I don't see a way to do that. I've seen other component exposes a prop like this, so I'd like to see it here too.

Environment

  • Package version(s): 4.14.3
  • Browser and OS versions: Chrome/Firefox Windows 11

Examples

<InputGroup
  className={styles.inputGroup}
  disabled={false}
  inputClassName={styles.input} // <-- this
  inputProps={{ className: 'xyz' }} // <-- or this
  name={name}
  placeholder='test'
  required={required}
  type='text'
  {...props} />
@adidahiya
Copy link
Contributor

@viveleroi do you mean on the <input> element or the leftIcon? your code snippet references "iconClassName"...

What are you trying to do exactly that is not possible with the existing APIs? It helps to provide a code sandbox demonstrating your exact scenario.

@viveleroi
Copy link
Author

Yes, the input. Sorry it was a typo saying "icon". I was jumping around the docs so much this weekend as I was using so many pieces.

I think the answer to my issue though is to use inputProps: { className: 'xyz' }, right?

@viveleroi
Copy link
Author

Nevermind the close, there is no inputProps. That's why I opened the issue.

@viveleroi viveleroi reopened this Jan 23, 2023
@viveleroi viveleroi changed the title Adds inputClassName prop to IInputGroupProps Add inputClassName or inputProps to IInputGroupProps Jan 23, 2023
@adidahiya
Copy link
Contributor

@viveleroi you don't need inputProps. All remaining props get forwarded to the <input> element. <InputGroup> is designed to behave very much like an <input>, with some added props available on top of the default HTML input APIs.

@adidahiya
Copy link
Contributor

If you want to target the input in CSS:

<InputGroup className="my-input" />
.my-input .bp4-input {
  // styles here
}

@viveleroi
Copy link
Author

We're using CSS modules as well as utility classes for typography and I need to apply two classnames directly to the element. composes can't work with a target selector like you have. I can't apply the typography class to a higher element because blueprint CSS overrides it.

@viveleroi
Copy link
Author

So how do you forward a prop that exists on the InputGroup? I need className to set the input element class. That exists in the group so it's not a "remaining prop"

@adidahiya
Copy link
Contributor

composes can't work with a target selector like you have.

That sounds like a limitation of your CSS architecture. Whatever CSS authoring language or tooling you choose to use on top of Blueprint, we're using plain CSS so it should be trivial to generate selectors with enough specificity to override Blueprint's styles.

Which text input styles are you specifically trying to override? Again, it would really help if there was a code sandbox or something we can look at together here.

Are you trying to override typography in all Blueprint inputs? Then you should likely be writing plain CSS and applying vendor overrides in a file that is not a CSS module.

Even with CSS modules, you can do this:

.my-input :global(.bp4-input) {
  // styles here
}

@viveleroi
Copy link
Author

I don't understand why it's just "I have limited architecture" when your other components offer exactly what I'm after, this one doesn't. I'm not setting up a codepen because this isn't a debate about my architecture, it's just one missing prop that I've seen you use elsewhere.

For example Select2 has inputProps and menuProps and popoverProps. Clearly I'm not asking for anything you haven't already done.

My use case is that I need to apply a classname to the input element, either via props or via composes. composes will not work with a non-local selector. I'm not duplicating all of our typography styles to target bp elements directly. In this specific case your css was overriding my font-size. You have global CSS which leaks into my app as well and I have to override, so I'm likely going to strip out your CSS file since I happen to not use it outside of popover/overlay stuff.

@adidahiya
Copy link
Contributor

it's just one missing prop that I've seen you use elsewhere. For example Select2 has inputProps and menuProps and popoverProps.

Yes, it's true that we offer many customization points for Blueprint components so that you can apply overrides & attributes to nested elements and components. We try to design these in a predictable way. Select2 has inputProps because you sometimes need to override input attributes, and it just so happens that className is one of the supported attributes there.

With <InputGroup>, we allow overriding <input> attributes by passing through most props, except for a few like className which are applied to the container element. It wouldn't really make sense to have an API like <InputGroup inputProps={ className }>, but I suppose one could make the argument for adding a prop named inputClassName to support the use cases you described. This hasn't come up before since there are good alternatives for targeting that input element with CSS (as I described in previous comments).

I would accept a PR to add a new prop <InputGroup inputClassName{...} />.

You have global CSS which leaks into my app as well and I have to override

Yes, this is to be expected for Blueprint's architecture as it stands now. We've been using simple, global CSS for many years and it's not something we can easily change. There is some discussion around this in #248. Gilad's comment in particular (#248 (comment)) illustrates some potential approaches / ways to think about styling on top of Blueprint.

@adidahiya adidahiya reopened this Jan 23, 2023
@adidahiya adidahiya changed the title Add inputClassName or inputProps to IInputGroupProps Add inputClassName to InputGroup props Jan 23, 2023
@viveleroi
Copy link
Author

Given how many different architectures there are these days it's hard to make assumptions. Sure, simple CSS can override stuff. It's odd that some architectures make this harder than it needs to be, but the benefits outweigh these issues.

I would expect css modules and css-in-js to grow more popular and likely soon exceeded by some new concept. CSS Modules is great for us but it's far from perfect - conceptually and technically. For example it runs as a postcss plugin but it's own import system doesn't, meaning we can't use other postcss plugins.

We're building a new app with a brand new architecture so we're ditching SASS, so it would be nice to someday see source files that are modular and accept CSS variables, that way I can pick and choose what elements I want in your lib.

StorybookJS complicates this because it has css that leaks into stories and overrides fonts we usually inherit, so we also need to be able to set font classes more specifically than traditional css.

It's all just a big mess! Allowing custom classes/props/even DOM for some of these elements really help bp be flexible.

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

Successfully merging a pull request may close this issue.

2 participants