Skip to content

Commit

Permalink
Fix :insert_mode => :button bug
Browse files Browse the repository at this point in the history
Setting :insert_mode to :button was causing a crash. Here was the
issue:

This gem was monkey patching sectionOptions to return the Ruby
object's `options` instance variable. However, sectionOptions was
being called from the Obj-C section initializer
(XLFormSectionDescriptor.formSectionWithTitle) BEFORE we were able to
set `options` on the ruby object. So it was

Why was this only an issue with the :button option? Well:
In that Obj-C initializer there's a call to canInsertUsingButton:
- https://github.com/xmartlabs/XLForm/blob/master/XLForm/XL/Descriptors/XLFormSectionDescriptor.m#L85
- https://github.com/xmartlabs/XLForm/blob/master/XLForm/XL/Descriptors/XLFormSectionDescriptor.m#L322-L325

That method is a two-part conditional. The first is 'is the insert
mode set to button?' and the second is 'do the section options indicate that
I can insert'. The second part was where the crash was occurring but
it was never getting there because of the first part was evaluating to
false.

The main change here is to update our `sectionOptions` monkeypatch
to use the ruby instance variable options if set but otherwise
fallback to the initial value.
  • Loading branch information
jeffcarbs committed Nov 20, 2015
1 parent d705e97 commit e6756c3
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 14 deletions.
13 changes: 13 additions & 0 deletions app/screens/test_form_screen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,19 @@ def form_data
}
]
},
{
title: 'Multi-value with XLFormSectionInsertModeButton',
name: 'multi',
options: [:insert, :delete, :reorder],
insert_mode: :button,
cells: [
{
title: 'Some text',
name: :some_text,
type: :text
}
]
},
{
title: 'Subcells',
name: :sub_cells,
Expand Down
25 changes: 15 additions & 10 deletions lib/ProMotion/XLForm/xl_form_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,17 @@ def cellForFormController(form_controller)
end

if self.cell && self.cell.respond_to?(:setup)
self.cell.setup(cell_data, form_controller)
self.cell.setup(cell_data, form_controller)
end
self.configureCellAtCreationTime
end
self.cell

self.cell
end
end

class XLFormSectionDescriptor
attr_accessor :section_data, :options
attr_accessor :section_data

def self.section(section_data)
title = section_data[:title]
Expand All @@ -81,21 +81,26 @@ def self.section(section_data)

section = XLFormSectionDescriptor.formSectionWithTitle(title, sectionOptions: options, sectionInsertMode: insert_mode)
section.section_data = section_data
section.options = options

section
end

def options=(value)
@options = self.class.parse_section_options(value)
@section_options = self.class.parse_section_options(value)
end

def options
@options
end
# Since `sectionOptions` is a property on the Objective-C class and not a
# Ruby method we can't use `super` to fallback when overriding the method.
# To achieve the same thing we create an alias and use that instead.
alias :originalSectionOptions :sectionOptions

# This property/method is used in the Objective-C initializer and is called
# before we ever have a chance to set @section_options so we need to be able
# to fallback to the original.
def sectionOptions
options
@section_options || originalSectionOptions
end
alias_method :options, :sectionOptions

def self.parse_section_options(options)
return section_options(:none) if options.nil?
Expand Down
4 changes: 2 additions & 2 deletions lib/ProMotion/XLForm/xl_form_screen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def enabled=(value)
def enabled?
!self.form.isDisabled
end

# dismiss keyboard
def dismiss_keyboard
self.view.endEditing true
Expand Down Expand Up @@ -283,7 +283,7 @@ def tableView(table_view, heightForRowAtIndexPath: index_path)
cell_class = cell.class
if cell_class.respond_to?(:formDescriptorCellHeightForRowDescriptor)
return cell_class.formDescriptorCellHeightForRowDescriptor(row)
elsif row.respond_to?(:cell_data) && row.cell_data[:height]
elsif row.respond_to?(:cell_data) && row.cell_data && row.cell_data[:height]
return row.cell_data[:height]
end
self.tableView.rowHeight
Expand Down
4 changes: 2 additions & 2 deletions spec/test_xlform_screen_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ def form_screen
view("Some help text").should.not.be.nil
end

it "contains 8 sections" do
views(UITableView).first.numberOfSections.should == 8
it "contains 9 sections" do
views(UITableView).first.numberOfSections.should == 9
end

it "contains 1 section with 8 fields" do
Expand Down

0 comments on commit e6756c3

Please sign in to comment.