Open kieranhogg opened 3 years ago
Below are my observations. (Note I'm just learning crispy, so these observations are to help others, who are familiar with crispy, trace the issue.)
I see the following files
crispy_forms/templates/bootstrap/layout/multifield.html
crispy_forms/templates/bootstrap3/multifield.html
crispy_forms/templates/bootstrap3/layout/multifield.html
crispy_forms/templates/bootstrap4/layout/multifield.html
crispy_forms/templates/uni_form/multifield.html
crispy_forms/templates/uni_form/layout/multifield.html
crispy-bootstrap5/templates/bootstrap5/layout/multifield.html
In crispy_forms/layout.py
the following initial portion of the class is:
class MultiField(LayoutObject):
"""MultiField container. Renders to a MultiField <div>"""
template = "%s/layout/multifield.html"
field_template = "%s/multifield.html"
It seems if both the layout and field templates are required, then this is a problem for bootstrap
, bootstrap4
, and bootstrap5
, which are all missing the field_template
file required by the MultiField
class.
Thought I'd post observations so that someone may provide further insights.
See additional info on crispy-forms... Open issue: Missing bootstrap4/multifield.html Pull Request: Fixes #478 bug about missing multifield template in bootstrap3 #591
Thanks for the report. Would you be in the market to open a pull request to add this missing template?
Thanks!
First, thank you for the invite to submit a PR.
I'll be honest, I'm still coming up to speed with git & GH regarding setting up a PR. (Restarting career in sw dev - like sipping from a firehose/water cannon).
Furthermore I need further study on the whole template operation - and where the two different multifield templates play a role. I'm not one to slam something in calling it done. (Maybe one just needs to copy the multifield from bs3 into bs4 and bs5, but I don't have the knowledge to say this is appropriate or not.)
There are really two different bugs regarding missing multifield - one in 'crispy' for contained pack bs4 and one in external pack 'crispy-bs5'.
Lastly since this was apparently missed, then it obviously needs to have a test written to assure that this bug does not appear again. The docs for crispy specify that tests need to be written for PR acceptance. Here again I need to come up to speed on how to write these tests.
I would venture to say that there needs to be some consideration given on how to set up tests. Specifically tests that apply to all templates, not just this one specific multifield.html template. Something that may span across 'crispy' and then extend into template packs like 'crispy-bs5'. Especially since 'crispy' has packs bs (assume bs2), bs3, bs4.
Crispy docs are specific in how to contribute and get a PR accepted, in part by Run the tests.
Yet the doc for creating your own template pack, How to create your own template packs attempts to make the concept of creating your own, less arduous by stating, Take it easy, don’t panic, we won’t need this many templates for our template pack.
These latter two paragraphs seem contradictory in concept. If a template pack is a derivative of another, then it seems that there should be some kind of derivative test system. There was a missing template pack in bs3, it was fixed. I assume that bs4 template pack was derived from bs3, but multifield was missed. That missing piece was then carried on into external pack crispy-bs5. (My basic assumption, not that I've read git histories.)
It would seem that there should be some kind of testing for derived packs. This is far beyond my knowledge at this point, I'll get there but not maybe in the timeframe that you need a PR.
..Otto
I'm not sure if it's an error in my template setup as I can't believe it's an actual bug, but it can't seem to find the template for the MultiField() help but all the other functions seem to work.
Example code:
Layout( 'Basic details', 'name', Field('description', rows=2), MultiField('default', 'active') )
Template-loader postmortem