fix(Input): adjust styling of Input component

[kuzhelov]

Introduced changes allowed to achieve the following goals
-Input component has similar presentational aspects to HTML input - for all the type values

---

TODO

• Changelog

More to follow

Currently the following (pseudo) tree is rendered by Radio

<ElementType ...>
  <Input type='radio' .. />
  {label}
</ElementType>

while, ideally, it should be just about providing wrapper over <Input type='radio' ... />. For that the following things (at the bare minimum) should be addressed

• label property should be introduced to Input component
• rendered tree of Radio should be something like

<Input type='radio' ...{this.props} ... />     // both 'as' and 'label' properties will be handled by Input

[levithomason]

I'm wary of this composition here. I think before we start to compose these two components we should first get the Radio's styles and accessibility working. My fear is that the logic, layout, and styles required for the radio will not make this kind of composition less desirable than using a vanilla input here.

[kuzhelov]

this puts a very important point about our vision for the components that use the same name and, essentially, semantics as their HTML counterparts.

My thought on that is that it seems to be reasonable to provide client with the same behavior and visual aspects as those are for HTML element in case if the same set of attributes is used, i.e.

<Input type='radio' />

// should be essentially equivalent to use of
<input type='radio'>

At the same time, those components are designed to introduce augmented functionaliy - so the set of the things that can do is a superset of their HTML analogs. For example, Input component is able to properly handle cases where label prop is provided:

<Input type='radio'  label='foo value' />    // renders proper HTML

Otherwise there would be too many surprises provided to client if she will start to consume our components. Suppose that there is an application where forms were used - and at some point its developer decides to integrate Stardust. Why? For example, to extend inputs with our clearable fuinctionality. What could be the first step? Replace input elements on Input components. But if this change would produce some drastic changes to the look and presentational aspects of the application, this will be a blocker for further progress.

And, if we agree on these points mentioned above, it seems to be reasonable approach to guarantee this semantics consistency by consuming our components inside other ones, so that any problems will be immediately visible.

---

Hope that I am rather clear with reasoning behind my thoughts on that. Please, share your opinion in return

[levithomason]

The defaultValue of type should be text instead. Otherwise, this default value behavior is not going to show up in the documentation.

[levithomason]

This change means this component no longer supports children. Though that might be an acceptable change (not sure of the reasoning behind it) the propTypes are still claiming that this component supports children.

Let's ensure our component features, examples, and propTypes are all consistent.

[kuzhelov]

removing children here is done by intent - frankly, I don't see any reason how this form of consuming component could be properly used:

<Input>Foo value</Input>

First question is what this one should be transformed into? Should children part be treated as a label's part? But in that case it will violate our agreement that children should be just put as component's child tree, with no modifications and decorations.

Should it be just pasted 'as is'? In that case this will be the resulting HTML tree:

<div ...>
  Foo value
</div>

This one looks like complete nonsence for input. Should we assume that it should be rendered as part of the input HTML element, like this one:

<input>
   Foo value
</input>

But in that case we are violating default value for as property (div). Should we change default value for as to input? - won't be possible, already, as general cases without children won't be properly handled then :(

So, these dilemmas had lead me to conclusion that this children case should be eliminated completely - and, as a second proof for that we could consider regular HTML input element - it doesn't accept children. As we are aiming to mimic HTML input element with ours Input, it seems to be a proper way to do that

[levithomason]

There was a very long conversation about the SUIR Input API which explains why there are children:
Semantic-Org/Semantic-UI-React#281 (comment)

Specifically, children were introduced to support these two cases:

There isn't a great shorthand API available to support these cases, as detailed in the conversation linked. Instead, we opted to support a single label and a single action with the shorthand API. If users needed to add two labels or two actions, they used children. In that case, the <input /> child is where the component places the actual HTML input, allowing you to choose what is on the left or right of the input.

Immediate Proposal

For now, I would not support children and I would not support two labels nor two actions. I would just keep it simple and consistent. We may need to address this API later to accommodate more advanced usages.

Possible Future Proposal

My latest thinking on this kind of component would say that the Input should not have any of these concerns (labels and buttons). Instead, it might be accomplished something like:

<Label attached='end' content='$' />
<Input attached='horizontally' />
<Label attached='start' content='.00' />

and

<Input attached='end' />
<Dropdown attached='horizontally' menu={...} value='articles' />
<Button attached='start' content='Search' />

[brianespinosa]

@levithomason also regarding the above, I am not sure if it is good to discuss here or to open an RFC for it... regarding the naming of the <Label> component, I think it has always introduced a lot of confusion when discussing inputs since there is a literal <label> tag which behaves differently than the currently named Label component. Not sure what everyone else's feelings are, but I think it would make a lot of sense to find a different name for the Label component. At least as it pertains to Input components.

I do like the other above proposed simplifications to the shorthand API.

[levithomason]

I would really like to first get the Radio working per our design requirements before attempting composition with the Input. I'm not seeing much advantage to requiring the Radio to bundle all the Input features opposed to using a vanilla input component.

[kuzhelov]

at the same time, if we are agreeing to eliminate Input composition - then I do have a question why we would need Label component here? to me it seems a bit counterintuitive, as Label semantically doesn't correspond to form's label

[kuzhelov]

let me merge just Input's parts, and lets proceed with discussion around Radio component afterwards

[codecov]

Codecov Report

Merging #127 into master will increase coverage by 0.17%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
+ Coverage   67.68%   67.86%   +0.17%     
==========================================
  Files         101      101              
  Lines        1371     1366       -5     
  Branches      263      261       -2     
==========================================
- Hits          928      927       -1     
+ Misses        441      437       -4     
  Partials        2        2

Impacted Files	Coverage Δ
...rc/themes/teams/components/Input/inputVariables.ts	14.28% <ø> (+0.95%)	⬆️
src/themes/teams/components/Input/inputStyles.ts	16.66% <0%> (-3.34%)	⬇️
src/components/Input/Input.tsx	85.18% <100%> (+6.85%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e543e7...a1e0267. Read the comment docs.