iommirocks / iommi

Your first pick for a django power cord
http://iommi.rocks
BSD 3-Clause "New" or "Revised" License
718 stars 47 forks source link

Form.fields_template #436

Closed berycz closed 1 year ago

berycz commented 1 year ago

enhancement by #408

berycz commented 1 year ago

I think this one needs your closer attention, I'm not really sure about some parts 1) I took an example from django and I'm using hidden_fields where I also put custom Field.hidden's - I'm not sure whether that is good solution or bad 2) also should there be the part.input.__html__() or part.__html__()? but the latter one has the wrappers, which may or may not be wanted imo 3) the "trailing" slash in format_html('<input type="hidden" name="{}" value="{}" />', k, v) might be removed imo, since field.input.__html__ does not add it either 4) or maybe instead of hardcoding the hiddens they might be somehow as Field.hidden, so then it would fix 1 and 2 5) can you imagine a need for passing groups as instances instead of .__html__()? but then how to do it? 6) if you accept this PR, then it might be good to put it in the docs imo

boxed commented 1 year ago

I think we should not respect the grouping feature in the template. It seems more reasonable to me to just assert that you don't have grouping. If you are doing manual layout then you are probably not using grouping.

I also suggest that iommi renders the hidden elements at the end outside the template. You always want them anyway, and you don't care about the layout for those, so we might as well take care of that.

berycz commented 1 year ago

and what do you think about the custom hidden fields (Field.hidden), should they be also outside template as part.input.__html__(), or just in context["fields"]?

boxed commented 1 year ago

Just input.__html__() seems reasonable to me.

berycz commented 1 year ago

I wonder if someone might actually need to put a hidden input in a specific place in html... like for some JS stuff, e.g. selectors (getting values from inputs placed only in a specific wrapper for some ajax requests) or maybe even changing type from hidden to text or whatever

I'm starting to not like the idea that you can add any hidden fields in your Form, but then you're not free to move them wherever you want in your template, and you can't actually put them in the template at all 🤔

Maybe in the end it would be better to say "If you define fields in the Form, no matter what type, you have to put it in the fields_template when you use it!" (so to leave them in context["fields"]) and then automatically add only those hidden inputs that are automatically generated from request.GET

boxed commented 1 year ago

Yea.. maybe. If the documentation for fields_template has a nice example that does the right thing that people can copy paste to work off of then it's fine. Something like:

{% for field in form.fields %}
    {% if field.hidden %}
        {{ field }}
    {% else %}
         <<stuff here>>
    {% endif %}
{% endfor %}
berycz commented 1 year ago

eh, how do I pass an album? :) also feel free to modify the docs