Summary

Allow ConfigForms to use fieldsets.

Review Request #9045 — Created June 29, 2017 and submitted July 13, 2017, 5:02 p.m.

Information

Owner

david

Repository

Djblets

Branch

release-0.10.x

Bugs

Depends On

Commit

886d3f8...

Reviewers

Groups

djblets

People

Description

This change updates things so that if a ConfigForm has fieldsets, they'll be
rendered correctly (as opposed to just having all the fields rendered together
as if the fieldsets didn't exist).

This has one small niggle in that if a person defines their own fieldsets, one
of those fieldsets must contain the form_target field. I've added a log
statement to help people figure out what went wrong if they didn't include
this.

Testing done:
- Ran unit tests.
- Used this in conjunction with a Review Board change.

Testing Done


Ran unit tests.
Used this in conjunction with a Review Board change.

Issues

Description	From	Last Updated
I don't really love duplicating all this. How about introducing a djblets/forms/templates/form_fieldset.html that works as a base for this and …	chipx86	July 7, 2017, 5:21 p.m.
The class= should probably be within the if {% fieldset.classes %}.	chipx86	July 7, 2017, 3:35 p.m.
Can you pass the request?	chipx86	July 7, 2017, 3:34 p.m.
Alphabetical order.	chipx86	July 7, 2017, 3:34 p.m.
We should use a module-level logger here.	brennie	July 13, 2017, 4:34 p.m.
form ?	brennie	July 13, 2017, 4:33 p.m.

flake8 passed.

JSHint passed.

djblets/configforms/templates/configforms/config_page_form.html (Diff revision 1)

The issue has been resolved. Show all issues

I don't really love duplicating all this. How about introducing a djblets/forms/templates/form_fieldset.html that works as a base for this and the admin one? It could take a extra_fieldset_classes variable that the admin one can pass down for module aligned, along with a template name that can be passed for the field name.

Similarly, we could have a base for the form_field.html.

We used to have a lot of copies of these rules, which was painful to clean up, so I'm hoping to keep it as tidy and consistent as possible.

djblets/configforms/templates/configforms/config_page_form.html (Diff revision 1)
The issue has been resolved. Show all issues
```
The class= should probably be within the if {% fieldset.classes %}.
```
djblets/configforms/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you pass the request?
```
djblets/static/djblets/css/config-forms.less (Diff revision 1)
The issue has been resolved. Show all issues
```
Alphabetical order.
```

Change Summary:

Reduce duplication between admin and config forms.

Commit:

42e3b14d521d8c1c97fa28f29e2abc6d3faa29ae

886d3f8f44a5481ed2500fade9e39f8565ff20fb

Diff:

Revision 2 (+56 -52)

Show changes

	djblets/configforms/views.py
	djblets/configforms/templates/configforms/config_page_form.html
	djblets/configforms/templates/configforms/config_page_form_field.html
	djblets/forms/templates/djblets_forms/form_field.html
	djblets/forms/templates/djblets_forms/form_fieldset.html
	djblets/forms/templates/djblets_forms/admin/form_field.html
	djblets/forms/templates/djblets_forms/admin/form_fieldset.html
	djblets/static/djblets/css/config-forms.less

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

djblets/configforms/views.py (Diff revision 2)
The issue has been resolved. Show all issues
```
We should use a module-level logger here.
```
djblets/forms/templates/djblets_forms/form_field.html (Diff revision 2)
The issue has been resolved. Show all issues
```
form ?
```

Status:: Completed
Change Summary:: Pushed to release-0.10.x (215bbc2)