Align options API for checkbox, select, and radio

[mattapperson]

Right now select needs an array of objects with the params of value and text, and for radio groups it is id and label.

[snide]

So I looked into this briefly.

Select options are formatted like so...

<option value={option.value} key={index}>{option.text}</option>;

where a checkbox would have an input like...

      <input
        className="euiCheckbox__input"
        type="checkbox"
        id={id}
        checked={checked}
        onChange={onChange}
        disabled={disabled}
      />

and a separate label like...

      <label
        className="euiCheckbox__label"
        htmlFor={id}
      >
        {label}
      </label>

I think this makes sense as it. Calling "text" label instead would seem odd, since it's not actually a label, it's just the text within the option, and "value" is the actual tag name of the option, the id is actually on the select itself, not the options.

@cjcenizal do you have any opinions here?

[cjcenizal]

I think I agree with @snide on this one. I think we've been leaning more towards a transparent component interface, in which we use HTML attributes as prop names if they're passed-through as such, as opposed to treating them as implementation details and hiding them behind abstract prop names. I think I can trace this back to our decision to pass-through all props as HTML attributes (example) if they aren't defined in propTypes.

In this case, I think our code follows this pattern except in the case of text and label. But even here, each one seems to make sense within its own context so I'm not sure I see the benefit of choosing a one-size-fits-all name. But I'm open to arguments that justify this + a proposal for a name which will work for both.

[bevacqua]

Speaking of which, when are we making our own select? 😅

[mattapperson]

I understand the rational, however I would argue a counterpoint of the fact that we are already way broken away from how raw HTML works by having an input group vs each checkbox/radio being it's own input. As such they are components with their own wrapped API already making this feel confusing (in my opinion at least).

If however the choice is made to push ahead with the existing API, I would highly recommend adding a warning to the component when it detects someone used the object structure of the opposing component so as to make debugging easier.

[cjcenizal]

having an input group vs each checkbox/radio being it's own input

So to try to explain our thought process a bit... we also export EuiRadio and EuiCheckbox as individual components in an effort to provide flexibility, to try to support the use case where you can specify each checkbox/radio individually as its own input. EuiRadioGroup and EuiCheckboxGroup are optional abstractions over this, which we thought would offer some convenience. When designing their APIs we thought it made sense to be true to the APIs of the underlying "primitives" -- they use id and label so the group components use them, too.

Given this, do you think we could rename or redesign anything here for improved clarity?

adding a warning to the component when it detects someone used the object structure of the opposing component

++! Great idea. I think this can be done using the arrayOf and shapeOf prop type checkers.

[mattapperson]

I think I see both sides of the argument equally at this point. So I would say regardless of what we do a warning should be added, otherwise its easy to typo it based on personal expectations and have no clear idea as to why it is broken.

Moving beyond technical arguments to reasons for my preference... I would say that using react components is an abstraction from native. As a consumer of the component I don't care at all if the underlying code is a CSS styled input or some crazy "over engineered" mess of divs pretending to be an input. So from that perspective mimicking HTML input APIs seems to be futile. I would - again personally - prefer an API that feels clean, uniform, and logical in and of itself, ignoring the underlying details. Perhaps saying "this api exists because thats how the browsers did it" is the same as saying "we do it this way because we always did it this way"? IDK, I am just rambling now... lol

[cjcenizal]

prefer an API that feels clean, uniform, and logical in and of itself, ignoring the underlying details

I originally had the same approach for React component interfaces. I'd prefix boolean props like "disabled" with "is", resulting in isDisabled, and essentially tried to create an interface over the HTML attribute interface. The problem I found was that I was just creating a new standard over an already-existing standard and after awhile I found myself spending a lot of time on something I just didn't feel I was getting much benefit from. Plus, doing this would make it much more difficult to simply pass through attributed using {...rest}. So in the end, there were clear benefits (to me) to giving up on this and not many downsides.

[snide]

I tend to agree, if for no other reason then it's too hard to name things and it's easier to just follow what's already there and what people should have some level of muscle memory for. Granted, we're not all writing checkboxes all day, so even something like this I tend to look up. As long as the docs are solid I'm cool with it as is.

[snide]

Closing this issue.