readthedocs.org
readthedocs.org copied to clipboard
Forms: avoid Crispy layout API?
In the work around the new dashboard and handling forms, I found a few forms where we decided to use the Crispy layout API instead of a basic Django form. This was usually to add fieldsets, but sometimes it was to add HTML content to the form. I'd like to discuss the merits of this pattern, and whether it's worth continuing or perhaps removing and settling on a single pattern.
The issues that I have with this pattern:
- All HTML display time code is in templates, except for these few forms. There are reusable patterns in templates that are not available in the layout API.
- Authoring HTML in Python is painful, and worse if we need JS/Knockout code in HTML in Python.
- There are some cases where a native FUI/SUI element is better than a FieldSet or similar. That is, we mostly want to add more manual HTML around fields, instead of stricter Crispy/Form widgets like FieldSet.
So, I personally would rather author complex forms in templates rather than in HTML in Python, as it is easier to maintain and author.
However, one point I'll give a pattern like the layout API is that we can get closer to rendering all forms with the same template logic: {% crispy form %}. This doesn't outweigh the negatives for me though.
Are there more benefits to using the Crispy layout API that I'm not acknowledging?
The two things I like about using Layout/FieldSet on Python are:
- it's a technology we all know and feel comfortable with
- it requires changing the code only in one place; there is no need to touch templates to add a new field, re-arrange them or similar
As I haven't touched the new templates too much, I don't have any benefits to highlight about not using the Crispy Layout pattern, tho. However, I'd like to know what would be the pattern you are suggesting to create something similar to Layout/FieldSet from the templates?
The next, or maybe even a previous question would be, "do we need these Layout/FieldSet at all?". It seems we were only using it for:
- Advanced setting (we don't need it anymore after https://github.com/readthedocs/readthedocs.org/pull/11036)
- User/Email settings (at https://beta.readthedocs.org/accounts/edit/ --not that it doesn't render the layout in beta templates, but it does on old ones https://readthedocs.org/accounts/edit/)
- Version edit (https://beta.readthedocs.org/dashboard/docs/version/latest/edit/ and https://readthedocs.org/dashboard/dev/version/latest/edit/): we don't probably need the layout anymore there either.
So, I'm 85% convinced we don't need this Layout/FieldSet pattern, but I'd like to know what is the replacement your are proposing in case we need it.
it's a technology we all know and feel comfortable with
You noted this too, but I might describe this pattern as less standardized than that. We've used it, but only in a few views. But also, we haven't absorbed doing complex FUI/SUI with the layout API yet either, which might feel a bit less comfortable :smile:
it requires changing the code only in one place
Yeah, I can see how from just the form/application side of things this would be nicer. I feel the opposite of this actually though, as working display side of things it's not as nice editing a template and coinciding application form simultaneously for the same HTML output.
However, I'd like to know what would be the pattern you are suggesting to create something similar to Layout/FieldSet from the templates?
There isn't a great answer here. The problem I struggle with is a form that is more complex than a very vanilla Django form (just a simple {{ form | crispy }}) requires you to manipulate the form instance somewhere -- so far this has been in template code.
This is the case for the following changes:
- Adding a Knockout binding to a field for a dynamic or interactive field. Currently, forms alter the field with bindings like this: https://github.com/readthedocs/ext-theme/blob/5dc73e2b8e120b4fde524c79dda99f1d3e754d00/readthedocsext/theme/templates/projects/redirect_form.html#L15-L17
- Using more FUI element structure could help build a really nice UI, as opposed to just a list of fields. This becomes all manual, and I would never attempt this much with the Crispy layout API: https://github.com/readthedocs/ext-theme/blob/5dc73e2b8e120b4fde524c79dda99f1d3e754d00/readthedocsext/theme/templates/organizations/settings/sso_edit.html#L44-L143 (I actually got so frustrated working with the underlying Form that I threw out all of this work)
- And for something like the Addons UI, where we want to divide up a complex form, I would either use multiple form instances on a FUI tabbed interface, or even multiple views/forms without a tabbed interface, over a very long, complex FieldSet form. Tabs or admin subpages would help isolate the amount of UI and information we're throwing at the user at once.
I think we're talking the same thing overall, and the examples you pointed out are on their way to removing the API already, but I also don't have a slam dunk replacement in templates. At very least, keeping all of the HTML in one place feels way more maintainable and easier to author though.
I understand the frustration that you are describing when working with forms and requiring specific HTML/JS code 😄
I don't have too much to add here, but I just wanted to say that I'm not in love with any of the patterns you liked to in the previous comment 😅 . Using custom alter_field template tag looks pretty similar to manage it by Python code in the form itself, since the alter_field is a Python function in the end. Also, it's a non-standard way to handle forms in Django, which makes me some noise.
On the other hand, don't rendering the form at all using {{ form|crispy }} and writing the whole HTML code is not necessarily bad, and I've seen it multiple times. However, it's pretty hard to maintain over time, get updates and parity with the rest of the forms, and I would avoid this pattern as much as possible.
I don't have too much to add here, but I just wanted to say that I'm not in love with any of the patterns you liked to in the previous comment
Yeah, neither pattern is great, but I do feel it is far better than having to add the HTML structure or HTML attributes like Knockout bindings in Python code. At least using template tags separates concerns and keeps all of the HTML changes in HTML/templates. I don't feel it matters too much that the logic ultimately comes from Python inside template code, that is almost unavoidable.
For some background, this was the same concept as: https://github.com/jazzband/django-widget-tweaks
writing the whole HTML code is not necessarily bad
I'd say it's not bad when there is reason to restructure the whole form, but to customize a single field this would be a bad pattern to follow. I share concerns around the form class and template drifting apart as well, and would only use this pattern when we really want a custom form.
Long term, I think this is maybe a point for web components. Web components are all JS driven, instead of using data binding attributes to dictate how elements behave. This could help reduce some of the concerns, though I don't really know what this pattern looks like yet either.
For now though, I still feel altering the fields in HTML is the best overall solution when we need to add CSS classes/data bindings to fields. I feel the Crispy API is nice in theory, but we don't have a strong reason to use it heavily, and the areas we are using it -- adding commentary in between fields and splitting up complex forms into multiple sections -- are patterns we should avoid in themselves.
But, again, these patterns are just compromises overall. I have not found a pattern for form HTML that I am excited to work with, just patterns I hate less.