willoma / bulma-gomponents

Bulma helpers for Gomponents
MIT License
10 stars 1 forks source link

b.Select not handling ID and Name atttributes correct - Parent DIV added #13

Closed chrismalek closed 1 month ago

chrismalek commented 1 month ago

I have this code and I am trying to add ID and Name attributes.


    selectField :=

            b.Select(

                e.Name(FieldTypeAnswerAnswer.GetField().Name),
                e.ID(FieldTypeAnswerAnswer.GetField().Name),
                b.Option("", "Select One"),
            )

        for val, description := range q.ValidValues.Map {
            selectField.With(
                b.Option(val, description.String),
            )

        }

The HTML that is generated is not correct. It is putting those attributes on the parent div NOT the delect.


<div class="select" name="answer" id="answer">
  <select>
    <option value="">Select One</option>
    <option value="UTC+02:00">South Africa Standard Time (Johannesburg, Cape Town)</option>
    <option value="UTC+04:00">Gulf Standard Time (Dubai, Abu Dhabi)</option>
    <option value="UTC+05:30">India Standard Time (Kolkata, Mumbai, New Delhi)</option>
    <option value="UTC+08:00">China Standard Time (Shanghai, Beijing, Guangzhou)</option>
    <option value="UTC-03:00">Brasilia Time (Sao Paulo, Rio de Janeiro)</option>
    <option value="UTC-05:00">Eastern Time Zone (New York, Boston, Washington D.C.)</option>
    <option value="UTC+00:00">Greenwich Mean Time (London, Dublin, Edinburgh)</option>
    <option value="UTC+01:00">Central European Time (Berlin, Paris, Rome)</option>
    <option value="UTC-08:00">Pacific Time Zone (Los Angeles, San Francisco, Seattle)</option>
    <option value="UTC-06:00">Central Time Zone (Chicago, Houston, Dallas)</option>
    <option value="UTC+09:00">Japan Standard Time (Tokyo, Osaka, Kyoto)</option>
    <option value="UTC+10:00">Australian Eastern Standard Time (Sydney, Melbourne, Brisbane)</option>
  </select>
</div>

Expected:


<div class="select" >
  <select name="answer" id="answer">
    <option value="">Select One</option>
    <option value="UTC+02:00">South Africa Standard Time (Johannesburg, Cape Town)</option>
    <option value="UTC+04:00">Gulf Standard Time (Dubai, Abu Dhabi)</option>
    <option value="UTC+05:30">India Standard Time (Kolkata, Mumbai, New Delhi)</option>
    <option value="UTC+08:00">China Standard Time (Shanghai, Beijing, Guangzhou)</option>
    <option value="UTC-03:00">Brasilia Time (Sao Paulo, Rio de Janeiro)</option>
    <option value="UTC-05:00">Eastern Time Zone (New York, Boston, Washington D.C.)</option>
    <option value="UTC+00:00">Greenwich Mean Time (London, Dublin, Edinburgh)</option>
    <option value="UTC+01:00">Central European Time (Berlin, Paris, Rome)</option>
    <option value="UTC-08:00">Pacific Time Zone (Los Angeles, San Francisco, Seattle)</option>
    <option value="UTC-06:00">Central Time Zone (Chicago, Houston, Dallas)</option>
    <option value="UTC+09:00">Japan Standard Time (Tokyo, Osaka, Kyoto)</option>
    <option value="UTC+10:00">Australian Eastern Standard Time (Sydney, Melbourne, Brisbane)</option>
  </select>
</div>
chrismalek commented 1 month ago

Sorry I figured it out. b.OnSelect function.

willoma commented 1 month ago

I wrote it this way so that when applying some alignment styling, etc, it would be applied to the div.

Do you think this behaviour should be the other way round? It may be more logical, for instance with Checkbox the children apply on the input by default, not on the label...

chrismalek commented 1 month ago

To be honest, I could argue it either way. With the current structure, you need to know how Bulma HTML is generated and that a wrapper div is needed for Bulma to represent this correctly.

I am not deep into the internals of Bulma, as I suspect you have written this great library.

From an outsider's perspective, when I write b.Select(foo), I am expecting b.select to actually apply to the actual HTML select element. Since it is scoped with the b., it is still doing a Bulma select which if you look closely at the Bulma docs you see there is a wrapper DIV. So I think it depends on the lens you are looking through here. Are you looking through the "HTML Lense" or the "Bulma Lense".

I am still working on my first cup of coffee when I write this, so I will just brainstorm here. Please take it with a grain of salt.

However, I think 1 or 2 code samples on the select documentation are probably more than enough. I saw those b.OnSelect in the docs, and I was expecting this interface b.Select().OnSelect(), but it was not there, so I thought the documents were wrong.

Of course, it was completely my fault for not digging in deeper. I'm juggling so many software projects that I can't keep them all in my head. I took a break from an app I was building a few months ago with this, and I am getting back into it, so expect to hear more from me. 👋🏻 (LOL).

In my case, the final code was the following. I was not clear from the docs that I just needed to wrap the stuff in 'onSelect` to tell it.

Part of me says this is "OK" and part of me thinks it is asking the client to understand the internals. It is not the end of the world. Another person might see the opposite from my perspective. I am just thinking out loud here and these are NOT strong opinions or an official request for a change. 😄

        selectField := b.Select(
            b.OnSelect(
                e.Name(FieldTypeAnswerAnswer.GetField().Name),
                e.ID(FieldTypeAnswerAnswer.GetField().Name),
                b.Option("", "Select One")))

        for val, description := range q.ValidValues.Map {
            selectField.With(
                b.Option(val, description.String),
            )