Opened 7 years ago

Last modified 6 years ago

#418 new enhancement

GeoExt.form.recordToField : handle nullable fields

Reported by: fvanderbiest Owned by: elemoine
Priority: major Milestone: 1.2
Component: GeoExt.form Version: trunk
Keywords: Cc:
State: Review


As discussed in, the following patch adds the ability to detect a NULL field in a form created with GeoExt.form.recordToField.

To achieve this, a checkbox is shipped with the generated field, inside a composite field.

Attachments (2)

gx_418_A0.diff (27.8 KB) - added by fvanderbiest 7 years ago.
gx_418_A1.diff (19.0 KB) - added by fvanderbiest 6 years ago.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by fvanderbiest

comment:1 Changed 7 years ago by fvanderbiest

tests pass, please review

comment:2 Changed 7 years ago by elemoine

  • State changed from Review to Needs more work

Thanks for the patch.

My review comments:


  • the API doc for recordToFieldOptions should provide a link to the GeoExt.form.recordToField doc
  • when calling recordToField no need to create an empty object if recordToFieldOptions, things will be handled correctly by the recordToField function


  • test_recordToField fails in both IE8 and FF4
  • I'm concerned with the formatting of the doc strings for the options parameter (we can deal with that after commit)
  • fieldLabel = label || name is now used for setting the field label, this changes the behavior when label is the empty string. I see no reason for changing the original behavior.
  • shouldn't we rely on GeoExt i18n for boxLabel?

comment:3 Changed 6 years ago by fvanderbiest

  • State changed from Needs more work to Review
  • Version changed from 1.0 to trunk

Thanks for the review Eric. Here's a better patch with tests (OK on FF7 / Chromium 12 / IE9).

Changed 6 years ago by fvanderbiest

comment:4 Changed 6 years ago by ahocevar

  • Milestone changed from 1.1 to 1.2

Batch move of tickets to finish the 1.1 milestone.

Note: See TracTickets for help on using tickets.