Browse files

Replace ConcepForm field form logic with FieldFormCollection

The collection handles rendering each field's control and the first
available chart. This logic may need to be improved or make it more
option-drive.
  • Loading branch information...
1 parent 33a5e57 commit e38f66c2a7cc23732d43eb701c8e14ab672388ef @bruth bruth committed May 6, 2013
Showing with 29 additions and 91 deletions.
  1. +8 −89 coffee/cilantro/ui/concept/form.coffee
  2. +21 −2 coffee/cilantro/ui/field/form.coffee
View
97 coffee/cilantro/ui/concept/form.coffee
@@ -8,40 +8,6 @@ define [
templates = c._.object ['form'], templates
- class ConceptContextManager
- constructor: (model, options={}) ->
- @model = model
- @options = options
-
- setContext: (context) ->
- # Fetch the branch relative to the concept.id
- if not (@context = context.fetch({concept: @model.id}, {deep: true}))
- # Create a branch-style node to put all field-level nodes inside,
- # mark it with the concept_id so it can be found later
- @context = new c.models.BranchNodeModel
- concept: @model.id
- type: 'and'
- children: []
-
- # Add to parent context
- context.add @context
- @model.context = @context
@jeffmax
DBHi member
jeffmax added a note May 7, 2013

Did this move somewhere else or are we not setting the context on the model anymore?

@bruth
DBHi member
bruth added a note May 7, 2013

The context is set down below on line 24 using the fetch method directly which now supports a create option which is true by default (see 4d8a278). This manager is now not necessary.

@bruth
DBHi member
bruth added a note May 7, 2013

That is coupled with ceedd3e which enables fetching by multiple key/value pairs, so one can do context.fetch({concept: 4, field: 10}) and it will get or create a node with those as the default attrs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- return
-
- getFieldContext: (id) ->
- if not (node = @context.fetch(field: id))
- # Create a branch-style node to put all field-level nodes inside,
- # mark it with the concept_id so it can be found later
- node = new c.models.ConditionNodeModel
- field: id
- concept: @model.id
-
- # Add to parent context
- @context.add node
-
- return node
-
-
class ConceptForm extends c.Marionette.Layout
className: 'concept-form'
@@ -52,11 +18,10 @@ define [
showChart: true
chartField: null
- constructor: (model) ->
- super model
- @manager = new ConceptContextManager(@model)
- # Base the context off the current session
- @manager.setContext(c.data.contexts.getSession())
+ constructor: ->
+ super
+ session = c.data.contexts.getSession()
+ @context = session.fetch(concept: @model.id)
@jeffmax
DBHi member
jeffmax added a note May 7, 2013

I was previously using the {deep:true} flag here because I have that requirement for the vocab browser. Is it ok to put that back?

@bruth
DBHi member
bruth added a note May 7, 2013
@bruth
DBHi member
bruth added a note May 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
events:
'click .concept-actions [data-toggle=add]': 'save'
@@ -75,57 +40,11 @@ define [
fields: '.concept-fields'
onRender: ->
- conceptFields = @model.fields[..]
-
- # Optionally render the chart if a distribution is available
- if @options.showChart
- idx = 0
-
- if (chartField = @options.chartField)?
- # Get the index of the field and ensure it exists for this
- # concept
- if (idx = conceptFields.indexOf(chartField)) is -1
- throw new Error('Field not associated with concept')
-
- # Throw an error for an explicitly defined field if it
- # does not support distributions
- if not chartField.urls.distribution?
- throw new Error('Field does not support distributions')
-
- if chartField or (chartField = @model.fields[idx]).urls.distribution?
- # Remove from remaining concept fields
- conceptFields.splice(idx, 1)
-
- context = @manager.getFieldContext(chartField.id)
-
- # TODO this could display it's own chart.. so there would
- # be no need to construct a separate chart here.
- mainFieldForm = new field.FieldForm
- showChart: false
- model: chartField
- context: context
-
- fieldChart = new charts.FieldChart
- parentView: @
- model: chartField
- context: context
-
- @main.show(mainFieldForm)
- @chart.show(fieldChart)
-
-
- fieldForms = new c.Marionette.CollectionView
- itemView: field.FieldForm
-
- # New collection for locally managing the fields..
- collection: new c.Backbone.Collection(conceptFields)
-
- itemViewOptions: (model) =>
- showChart: false
- context: @manager.getFieldContext(model.id)
-
- @fields.show(fieldForms)
+ fields = new field.FieldFormCollection
+ collection: @model.fields
+ context: @context
+ @fields.show(fields)
@setDefaultState()
setDefaultState: ->
View
23 coffee/cilantro/ui/field/form.coffee
@@ -55,7 +55,7 @@ define [
model: @model
context: @context
@[key].show view
-
+
# Only represent for fields that support distributions
if @options.showChart and @model.urls.distribution?
chart = new charts.FieldChart
@@ -95,4 +95,23 @@ define [
if @options.managed then @setNewState()
- { FieldForm }
+ class FieldFormCollection extends c.Marionette.CollectionView
+ itemView: FieldForm
+
+ itemViewOptions: (model, index) ->
+ context = @options.context
+
+ options =
+ model: model
+ context: context.fetch
+ field: model.id
+ concept: context.get 'concept'
+
+ if not @fieldChartIndex? and model.urls.distribution?
+ @fieldChartIndex = index
+ options.showChart = true
+
+ return options
+
+
+ { FieldForm, FieldFormCollection }

0 comments on commit e38f66c

Please sign in to comment.