Addons: leave Crispy forms to handle the rendering of the form

[humitos]

This is the correct way to render a Crispy form. It's also required for https://github.com/readthedocs/ext-theme/issues/211

[agjohnson]

What does the form look like when rendered this way? The form|crispy was most likely intentional, so we can control the additional elements like the submit button.

[humitos]

Oh, I saw that we were using {% crispy form %} for a form that has a custom Layout: https://github.com/readthedocs/readthedocs.org/blob/919d10aca54650c4a9af5f597f8a0f08646d3206/readthedocs/projects/forms.py#L280-L292 and https://github.com/readthedocs/ext-theme/blob/a39cec9063a82d55e1a549f914d14c90dbb2e506/readthedocsext/theme/templates/projects/settings_advanced_form.html#L10-L15.

I followed that pattern because it does not render properly otherwise. The following screenshot is using origin/main branch:

Using this PR, it renders the layout properly as shown in https://github.com/readthedocs/readthedocs.org/pull/11031

The documentation of crispy form says that using the filter is fine, but the tag gives a lot more customization power: https://django-crispy-forms.readthedocs.io/en/latest/filters.html

The best way to make your forms crisp is using the {% crispy %} tag with forms.

and it's the required way to use layouts as I want to do here:

django-crispy-forms implements a class called FormHelper that defines the form rendering behavior. Helpers give you a way to control form attributes and its layout, doing this in a programmatic way using Python

_(from https://django-crispy-forms.readthedocs.io/en/latest/crispy_tag_forms.html#crispy-tag-forms)_

so we can control the additional elements like the submit button.

We can control this as well, but instead of doing it at the template level, it's done in Python: https://github.com/readthedocs/readthedocs.org/blob/919d10aca54650c4a9af5f597f8a0f08646d3206/readthedocs/projects/forms.py#L292

[agjohnson]

Yeah, I wanted to eventually discuss this pattern. I know we used Layout in a few forms (project advanced settings included), but I'm a little hesitant to continue relying on this feature or any part of Django form/widget handling that requires writing HTML in Python, or worse, JS/Knockout in HTML in Python. Mostly because it's awful switching between Python and template code to change HTML rendering, and even worse when JS/Knockout gets added to the form.

I think fieldset separation is okayish though, but we should probably set a firm line in the patterns we're relying on.

[agjohnson]

Also, it's probably worth doing this in the simplest way possible for now. I am going to want to split this up into a tabbed interface or multiple subpages, so we most likely will need multiple forms/views eventually (instead of fieldsets). I would plan for this change in the future with the code you are writing now, probably by avoiding planning out the UI too much to start.

I'll note this on https://github.com/readthedocs/readthedocs.org/pull/11031

Also also, this is rendering a little extra bad right now as I recently fixed a bug in checkbox display. I was wondering if this affected any of the checkboxes but didn't see any worse off. But it does look like multiple checkboxes are rendering inline instead of as block elements. I'll need a fix for this, which would help render this at least as a single list of checkboxes.

[humitos]

OK. I don't want to change the pattern you were following (or the one you want to follow). I will check your review from that PR, but I'm happy to adapt this work to keep it simple and move forward with it. Currently, I just want to start giving users a way to enable/disable each addon and be able to promote/discover it more.

[agjohnson]

And let's continue discussion around forms and the Crispy layout API at https://github.com/readthedocs/readthedocs.org/issues/11042. I find it's easier to author, manipulate, and style forms inside templates, but if there are merits for using the layout API instead, I'd like that input too.

• https://github.com/readthedocs/readthedocs.org/issues/11042