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 meaningful CSS class to AutosizeInput wrapper div #4573

Conversation

trumbitta
Copy link

This little div:

<div
  class="select__value-container select__value-container--has-value css-g1d714-ValueContainer"
>
  <div class="select__single-value css-1uccc91-singleValue">Ocean</div>

  <!-- This one right here below -->
  <div class="css-b8ldur-Input">

    <div class="select__input" style="display: inline-block">
      <input [...] />
      [...]
    </div>
  </div>
</div>

bears some CSS rules that would be much easier to customize (via class name / styled-components) if it had a proper, meaningful, fixed class name.

This little PR adds that class name:

<div
  class="select__value-container select__value-container--has-value css-g1d714-ValueContainer"
>
  <div class="select__single-value css-1uccc91-singleValue">Ocean</div>

  <!-- This one right here below -->
  <div class="css-b8ldur-Input select__input-wrapper">

    <div class="select__input" style="display: inline-block">
      <input [...] />
      [...]
    </div>
  </div>
</div>

Without it, I'm having to do this:

    /*
      This is the parent of the div that hides the real HTML input element.
      The problem is that this element causes unwanted margin, height, padding but
      it has no proper name / class while its direct child has a proper name / class but at the time
      of writing we can't target a parent by its child in CSS (https://caniuse.com/css-has)

      So, I guess I'll just 🤞
    */
    div:nth-child(2) {
      margin: 0;
      padding: 0;
    }

but I would rather not :D

@changeset-bot
Copy link

changeset-bot bot commented May 7, 2021

⚠️ No Changeset found

Latest commit: 562e9a1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 7, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 562e9a1:

Sandbox Source
react-codesandboxer-example Configuration

@trumbitta trumbitta force-pushed the add-meaningful-css-class-to-control-container-sub-div branch from 45b99a6 to 537ce8c Compare May 7, 2021 14:06
@JedWatson
Copy link
Owner

Hey @trumbitta thanks for this PR!

We're currently working through the updates we're making for 5.0 (see #4559) and have two thoughts on this:

  1. We are looking into replacing react-input-autosize with a css-only solution now that we have dropped IE11 support, which would clean this whole part of the component up considerably. If we do that, we may be able to remove one of these two wrapping divs entirely
  2. If we do keep both wrapping divs, I'd be very happy to merge this PR, but for consistency with other wrapper class names we should change it to input-container and not input-wrapper

Will update you shortly with how this is going

@trumbitta trumbitta force-pushed the add-meaningful-css-class-to-control-container-sub-div branch from 537ce8c to 562e9a1 Compare May 18, 2021 09:22
@trumbitta
Copy link
Author

Hey @JedWatson thanks for the heads-up!

While you decide about the best course of action, I thought I'd better help by doing what I could and so I updated the name to input-container and rebased the branch to get the latest updates from master in.

@ebonow
Copy link
Collaborator

ebonow commented Jun 16, 2021

@trumbitta just wanted to say thanks for this PR.

One of the goals moving into a new major version was to identify any breaking changes and autosizeinput was one of those long overdue changes. Much of the Input component was thus overhauled as a part of this PR. #4625

The impact of the referenced PR is that it duplicates this functionality.

Some other things to note:

  • The DOM structure was simplified so the Input component is only one div wrapping an input
  • inputClassName was added as a prop to allow more control in adding styles to the input
  • .prefix__input-container refers to the div wrapper whereas .prefix__input will now refer to the input element. This will likely cause some initial confusion, but feel it's much more consistent with the styles.

That all said, I agree with the intention of this PR, but will close this as this functionality will is included as a part of another PR.

@ebonow ebonow closed this Jun 16, 2021
@trumbitta
Copy link
Author

Hey, thanks for the heads-up!

I'm excited for the upcoming release, but at the same time I don't know when I will be able to use it since a sizable slice of our users unfortunately still uses IE11.

Is there any chance you'll release a patch version with this PR before going forward with the next major? 🤞🤞🤞

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