Summary

Fix rendering for config form JS models

Review Request #8082 — Created March 28, 2016 and submitted March 29, 2016, 7:53 p.m.

Information

Owner

brennie

Repository

Djblets

Branch

release-0.8.x

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

The JS model for a JS view for a config form was previously rendering
incorrectly (without a trailing )). This issue has been addressed.
Also, we now are more careful to only render a trailing comma after the
JS model data if there is in fact a page-wide config object. This object
is now only added to the models if it exists.

Testing Done

Tested this with Review Board. The generated JS works now.

Issues

Description	From	Last Updated
I think you broke this line (missing a }})	david	March 28, 2016, 5:06 p.m.
Do we not want this line in the if form_model_data and js_model_class case?	david	March 28, 2016, 5:06 p.m.
We have a conditional around form.js_model_class on line 87--how is that different from js_model_class?	david	March 28, 2016, 7:03 p.m.

Tool: PEP8 Style Checker
Ignored Files:
    djblets/configforms/templates/configforms/config.html



Tool: Pyflakes
Ignored Files:
    djblets/configforms/templates/configforms/config.html

Diff:

Revision 2 (+10 -5)

Show changes

djblets/configforms/templates/configforms/config.html

Tool: PEP8 Style Checker
Ignored Files:
    djblets/configforms/templates/configforms/config.html



Tool: Pyflakes
Ignored Files:
    djblets/configforms/templates/configforms/config.html

```
Typo in description: "preivously"
```
djblets/configforms/templates/configforms/config.html (Diff revision 2)
The issue has been resolved. Show all issues
```
I think you broke this line (missing a }})
```
djblets/configforms/templates/configforms/config.html (Diff revision 2)
The issue has been resolved. Show all issues
```
Do we not want this line in the if form_model_data and js_model_class case?
```

Change Summary:

Address David's issues

Description:

~		The JS model for a JS view for a config form was preivously rendering
	~	The JS model for a JS view for a config form was previously rendering
		incorrectly (without a trailing `)`). This issue has been addressed.
		Also, we now are more careful to only render a trailing comma after the
		JS model data if there is in fact a page-wide config object. This object
		is now only added to the models if it exists.

Diff:

Revision 3 (+13 -5)

Show changes

djblets/configforms/templates/configforms/config.html

Tool: PEP8 Style Checker
Ignored Files:
    djblets/configforms/templates/configforms/config.html



Tool: Pyflakes
Ignored Files:
    djblets/configforms/templates/configforms/config.html

djblets/configforms/templates/configforms/config.html (Diff revision 3)

The issue has been dropped. Show all issues

We have a conditional around form.js_model_class on line 87--how is that different from js_model_class?

brennie March 28, 2016, 7:03 p.m.

The ConfigPagesView class has a js_model_class, which is used for maintaining the state of the entire page -- that is the js_model_class template variable.
The form.js_model_class is the ConfigPageForm.js_model_class attribute, i.e., the model for an individual form.

Change Summary:

Formatting

Diff:

Revision 4 (+12 -4)

Show changes

djblets/configforms/templates/configforms/config.html

Tool: Pyflakes
Ignored Files:
    djblets/configforms/templates/configforms/config.html



Tool: PEP8 Style Checker
Ignored Files:
    djblets/configforms/templates/configforms/config.html

Diff:

Revision 5 (+12 -4)

Show changes

djblets/configforms/templates/configforms/config.html

Tool: Pyflakes
Ignored Files:
    djblets/configforms/templates/configforms/config.html



Tool: PEP8 Style Checker
Ignored Files:
    djblets/configforms/templates/configforms/config.html

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-0.8.x (206ee25)