jaspervdj / digestive-functors

A general way to consume input using applicative functors
149 stars 71 forks source link

Heist: add wrapping div around label/input pairs for dfInputRadio #60

Closed dbp closed 11 years ago

dbp commented 11 years ago

The way the current code is, the labels / inputs are just sent out one after another, which makes it really hard to style, as you can't apply styling to the pairs as units, because the markup looks like:

<input ...>
<label ...>
<input ...>
<label ...>
...

So I added a <div class="radio"> around each pair, so it looks like

<div class="radio">
    <input ...>
    <label ...>
</div>
<div class="radio">
    <input ...>
    <label ...>
</div>
...

Which can by styled as inline, if desired.

ghost commented 11 years ago

I really don't like the idea of hard coding particular markup into the splice. I think it should be possible to make radios work with heist nicely using mapSplices shouldn't it? So that everyone can add whatever markup they want in their template (I'd like to use li instead of div for example).

dbp commented 11 years ago

Well, there is already hard-coded markup for the error list (it is a ul).

Redoing them both with mapSplices/runChildrenWith would work, and might makes sense... Though it might be convenient to keep a version of the splices that just gives you defaults (as the markup will be bigger).

Another option could be to parametrize radio (and error list) with what the wrapping tag is. Either put the parameter on the splice, so you use a different set of splices, or actually just do it in the tag, ie <dfInputErrorList wrap="div"...>.

But for the radio, I don't think it is usable without some wrapping tag (whether it be span, div, li, p).

cimmanon commented 11 years ago

In the case of a collection of radios, a list seems like the most appropriate markup because they are a list of options for a specific field. Some might disagree with that interpretation of the data and opt to throw them into a fieldset and call it a day.

The simplest markup that would satisfy those of us who want to style the radio+label elements as a pair and those of us who never want to use a div if they can help it would be to place the radio in the label:

<label><input type="radio" /> Label Text</label>

By default, these elements would appear inline. If the elements need to be displayed vertically (my preferred way of displaying radio elements, as it reduces ambiguity for the user; plenty of people don't get how the label element functions even when it is provided), then a touch of CSS can fix it:

label {
    display: table; /* or list-item or block */
}

With the way the markup is now, I agree that it is what I would consider to be unusable once there's more than 2 options for usability reasons. I'd prefer to have the markup be fully customizable, but I'd settle for just plain usable. Moving the input inside the label would be a quick fix and move it into usable column in my book for a majority of my use cases. It still won't be useful in cases where the label "text" is meant to be an image (eg. payment options: visa, mastercard, etc.).

dbp commented 11 years ago

@cimmanon - that actually sounds totally reasonable. Unless there are objections / other ideas, I'll push that change to this request.

jaspervdj commented 11 years ago

Just checking -- @mightybyte, does this work with your various use cases? It looks good to merge to me.

mightybyte commented 11 years ago

@jaspervdj I don't see anything wrong with it off-hand. I don't think we're using that splice at all, so you should be good to go.

jaspervdj commented 11 years ago

Merged. Thanks for the awesome patch! I just uploaded it to Hackage as digestive-functors-heist-0.6.2.0.