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

Input refactoring. Split select, radio, textarea, text-input #342

Closed
5 of 12 tasks
ivan-kleshnin opened this issue Jan 22, 2015 · 10 comments
Closed
5 of 12 tasks

Input refactoring. Split select, radio, textarea, text-input #342

ivan-kleshnin opened this issue Jan 22, 2015 · 10 comments

Comments

@ivan-kleshnin
Copy link

Currently they are in one codebase and the code is contaminated with multiple if statements...
I think it's worth splitting onto two, three or more distinct components. react-bootstrap is, perhaps, the only one that merged them together. My anti-favorite is <Input type="select"/> :(

-- EDIT by @mtscout6 --

I originally wrote down these thoughts against #205, that issue has been resolved, but these notes apply better here.

I think we should extract smaller components, where some are not publicly exposed.

We could add these components publicly:

@ivan-kleshnin Correct me if I missed one.

The public components would make use of these private components:

Each control than can should implement a getValue() function.

Since checkboxes do not correlate with each other the way radios do with the name property, I'm thinking they'd be different group types. This gets especially difficult when figuring out how to implement a getValue() function for the two different groups.

Example form when all is said and done:

class MyForm extends React.Component {
  render() {
    return (
      <form>
        {/* type prop defaults to 'text' */}
        <Input label='Text' placeholder='Enter text' />
        <Input type='email' label='Email' placeholder='Enter email' />
        <TextArea label='Text Area' placeholder='textarea' />
        {/* inline prop optional */}
        <CheckboxGroup inline>
          <Checkbox label='Package 1' checked />
          <Checkbox label='Package 2' />
          <Checkbox label='Package 3' checked readonly />
        </CheckboxGroup>
        {/* inline prop optional */}
        <RadioGroup name='radio-name' checked='option1' inline>
          <Radio label='Option 1' value='option1' />
          <Radio label='Option 2' value='option2' />
          <Radio label='Option 3' value='option3' />
        </RadioGroup>
        {/* multiple prop optional */}
        <Select label='Select' placeholder='select' multiple>
          <option value='select'>select</option>
          <option value='other'>...</option>
        </Select>
        <Static label='Static Text'>Static value</Static>
        <Submit>Submit button</Submit>
      </form>
    );
  }
}

I know this introduces a lot of work and really isn't a quick solution, but I feel that something like this would allow the flexibility that everyone is looking for, and significantly simplify what is now a very large component.

@mtscout6
Copy link
Member

👍

@mtscout6
Copy link
Member

EDIT Moved this comment to issue description in edit clause

@ivan-kleshnin
Copy link
Author

  1. What is Static?

  2. My wish would be to have the same API for interchangeable elements:
    RadioGroup <=> Select
    CheckboxGroup <=> MultiSelect

@mtscout6
Copy link
Member

Consider react-select when dealing with this. That's not a declaration that we will use it, only that we will review it. @binarykitchen Consideration as per #544

@binarykitchen
Copy link

ok, fair enough, thanks

@mtscout6
Copy link
Member

mtscout6 commented May 6, 2015

From conversation in #618:

render() {
  return (
    <Input type='text'>
        <AddonBefore className='custom-class' />
        <ButtonBefore className='custom-class' onClick={(event) => { ... } />
        {/* OR for dropdown variations */}
        <ButtonBefore className='custom-class'>
          <MenuItem>Dropdown menu item</MenuItem>
        </ButtonBefore>
        <ButtonAfter />
        <AddonAfter />
    </Input>
  );
}

I know it's not quite intuitive at first, but it really opens the doors for extensibility. Should we do this with ES6 classes than you could extend any of those components and override its render call. I don't know if this is possible with Bootstrap but it would allow you to, for example, have two buttons before the input.

We could do some validation logic to warn if you use an after component prior to any befores.

/cc @cymen

@aabenoja
Copy link
Member

aabenoja commented May 6, 2015

This is definitely better. (You already know my feelings about the 60 properties on the Input component.) I'll do some investigation into having multiple of the same component type.

@jondeandres
Copy link

Hey! Is there any progress on this?

I'd like to add a extra class name to this:

// In InputBase.js.

var addonBefore = this.props.addonBefore ? _react2['default'].createElement(
      'span',
      { className: 'input-group-addon', key: 'addonBefore' },
      this.props.addonBefore
    ) : null;

So I can have class="input-group-addon extra-class", for ex.

@aabenoja
Copy link
Member

@jondeandres Sorry, I don't have a timetable for when the rewrite of the add-ons. Swamped with work and I just got back from vacation.

@taion
Copy link
Member

taion commented Mar 31, 2016

Replacing with #1749.

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

No branches or pull requests

6 participants