This change has been marked as completed.

Describe the completed change (optional):

Pushed to master (28f8a36)

Summary

Added showRequiredFlag option to inlineEditor

Review Request #2889 — Created Feb. 17, 2012 and submitted 13 years, 2 months ago

Information

Owner

medanat*

Repository

Djblets

Branch

Bugs

Depends On

Reviewers

Groups

djblets

People

Description*

Added showRequiredFlag option to inlineEditor.

This review is part of review 2881.

Testing Done

Issues

Description	From	Last Updated
This can be combined to one statement. The indentation for the values on the $("") should be indented one more …	chipx86	13 years, 2 months ago
This should be indented 2 more spaces.	mike_conley	13 years, 2 months ago
There are no open issues

Status:: Discarded

Status:: Re-opened
Summary:: Added showRequiredFlag option to inlineEditor
Description:: +
Added showRequiredFlag option to inlineEditor.
+
+
This review is part of review 2881.
Groups:: djblets
Diff:: Revision 1 (+10)

djblets/media/js/jquery.gravy.js

djblets/media/js/jquery.gravy.js (Diff revision 1)

The issue has been resolved. Show all issues

This can be combined to one statement.

The indentation for the values on the $("<span/>") should be indented one more level, as they apply to the span.

Change Summary:

Indented values for the span element. 
Combined code into one statement.

Diff:

Revision 2 (+8)

Show changes

djblets/media/js/jquery.gravy.js

Just one nit, Yazan.  Other than that, this looks good to me (assuming we decided that the * was an OK indicator for required fields)

djblets/media/js/jquery.gravy.js (Diff revision 2)
The issue has been resolved. Show all issues
```
This should be indented 2 more spaces.
```

Change Summary:

Fixed indentation.

Diff:

Revision 3 (+8)

Show changes

djblets/media/js/jquery.gravy.js

Ship it!

Looks good, though you're using tabs instead of spaces. Be careful about that in future changes. I've fixed it in the patch and will commit to master.

Status:: Completed
Change Summary:: Pushed to master (28f8a36)

	+	Added showRequiredFlag option to inlineEditor.
	+
	+	This review is part of review 2881.