From ad933168cbc71896c84a8649fbdba42cad75e787 Mon Sep 17 00:00:00 2001 From: Jamon Holmgren Date: Mon, 15 Sep 2014 17:07:17 -0700 Subject: [PATCH 1/6] Warn when attempting to set an invalid property using set_attributes. --- lib/ProMotion/styling/styling.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/ProMotion/styling/styling.rb b/lib/ProMotion/styling/styling.rb index 169ecb8..79f6faf 100644 --- a/lib/ProMotion/styling/styling.rb +++ b/lib/ProMotion/styling/styling.rb @@ -18,9 +18,10 @@ module ProMotion element.send("#{k}=", v) elsif v.is_a?(Array) && element.respond_to?("#{k}") && element.method("#{k}").arity == v.length element.send("#{k}", *v) - else - # Doesn't respond. Check if snake case. - set_attribute(element, camelize(k), v) if k.to_s.include?("_") + elsif k.to_s.include?("_") # Snake case? + set_attribute(element, camelize(k), v) + else # Warn + PM.logger.warn "set_attribute: #{element.inspect} does not respond to #{k}=" end element end From e4611f8b88c3dbcb82505596c475f85e46d483c7 Mon Sep 17 00:00:00 2001 From: Jamon Holmgren Date: Mon, 15 Sep 2014 17:44:20 -0700 Subject: [PATCH 2/6] Revamped logger to be very verbose on :verbose, added whitelist for screen instantiation --- lib/ProMotion/logger/logger.rb | 9 +++++++-- lib/ProMotion/screen/screen_module.rb | 7 ++++++- lib/ProMotion/styling/styling.rb | 3 ++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/ProMotion/logger/logger.rb b/lib/ProMotion/logger/logger.rb index 3a63ca0..0d255e1 100644 --- a/lib/ProMotion/logger/logger.rb +++ b/lib/ProMotion/logger/logger.rb @@ -19,12 +19,17 @@ module ProMotion error: [:error], warn: [:error, :warn], info: [:error, :warn, :info], + debug: [:error, :warn, :info, :debug], verbose: [:error, :warn, :info, :debug, :verbose], - debug: [:error, :warn, :info, :debug, :verbose] } def level - @level ||= :debug + @level ||= (RUBYMOTION_ENV == "release" ? :error : :debug) + end + + def level=(new_level) + log('LOG LEVEL', 'Setting PM.logger to :verbose will make everything REALLY SLOW!', :purple) if new_level == :verbose + @level = new_level end def levels diff --git a/lib/ProMotion/screen/screen_module.rb b/lib/ProMotion/screen/screen_module.rb index 2756907..58f1a63 100644 --- a/lib/ProMotion/screen/screen_module.rb +++ b/lib/ProMotion/screen/screen_module.rb @@ -12,7 +12,7 @@ module ProMotion check_ancestry resolve_title tab_bar_setup - set_attributes self, args + apply_properties(args) add_nav_bar(args) if args[:nav_bar] try :screen_setup try :on_init @@ -146,6 +146,11 @@ module ProMotion private + def apply_properties(args) + reserved_args = [ :nav_bar, :hide_nav_bar, :hide_tab_bar, :animated, :close_all, :modal, :in_tab, :in_detail, :in_master, :to_screen ] + set_attributes self, args.dup.delete_if { |k,v| reserved_args.include?(k) } + end + def tab_bar_setup self.tab_bar_item = self.class.send(:get_tab_bar_item) self.refresh_tab_bar_item if self.tab_bar_item diff --git a/lib/ProMotion/styling/styling.rb b/lib/ProMotion/styling/styling.rb index 79f6faf..d47c878 100644 --- a/lib/ProMotion/styling/styling.rb +++ b/lib/ProMotion/styling/styling.rb @@ -21,7 +21,8 @@ module ProMotion elsif k.to_s.include?("_") # Snake case? set_attribute(element, camelize(k), v) else # Warn - PM.logger.warn "set_attribute: #{element.inspect} does not respond to #{k}=" + PM.logger.warn "set_attribute: #{element.inspect} does not respond to #{k}=." + PM.logger.log("BACKTRACE", caller(0).join("\n"), :default) if PM.logger.level == :verbose end element end From 88b9e75474197811e220453c80f8919d789439e6 Mon Sep 17 00:00:00 2001 From: Jamon Holmgren Date: Mon, 15 Sep 2014 18:02:25 -0700 Subject: [PATCH 3/6] Use debug mode, not warn --- lib/ProMotion/styling/styling.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ProMotion/styling/styling.rb b/lib/ProMotion/styling/styling.rb index d47c878..fbd715f 100644 --- a/lib/ProMotion/styling/styling.rb +++ b/lib/ProMotion/styling/styling.rb @@ -21,7 +21,7 @@ module ProMotion elsif k.to_s.include?("_") # Snake case? set_attribute(element, camelize(k), v) else # Warn - PM.logger.warn "set_attribute: #{element.inspect} does not respond to #{k}=." + PM.logger.debug "set_attribute: #{element.inspect} does not respond to #{k}=." PM.logger.log("BACKTRACE", caller(0).join("\n"), :default) if PM.logger.level == :verbose end element From 69f777c57d19b835ca293bb4f8312fd7b520f4e3 Mon Sep 17 00:00:00 2001 From: Jamon Holmgren Date: Mon, 15 Sep 2014 18:26:38 -0700 Subject: [PATCH 4/6] Fix regression - modal= should indeed be applied by set_attributes --- lib/ProMotion/screen/screen_module.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ProMotion/screen/screen_module.rb b/lib/ProMotion/screen/screen_module.rb index 58f1a63..c32a6a7 100644 --- a/lib/ProMotion/screen/screen_module.rb +++ b/lib/ProMotion/screen/screen_module.rb @@ -147,7 +147,7 @@ module ProMotion private def apply_properties(args) - reserved_args = [ :nav_bar, :hide_nav_bar, :hide_tab_bar, :animated, :close_all, :modal, :in_tab, :in_detail, :in_master, :to_screen ] + reserved_args = [ :nav_bar, :hide_nav_bar, :hide_tab_bar, :animated, :close_all, :in_tab, :in_detail, :in_master, :to_screen ] set_attributes self, args.dup.delete_if { |k,v| reserved_args.include?(k) } end From 6670898bd036b799af3ccfe58657011b5e732c21 Mon Sep 17 00:00:00 2001 From: Jamon Holmgren Date: Sat, 11 Oct 2014 12:57:23 -0700 Subject: [PATCH 5/6] Extensive work on tests --- Rakefile | 6 ++-- app/test_screens/functional_screen.rb | 5 +++ app/test_screens/navigation_screen.rb | 5 ++- .../cocoatouch/navigation_controller.rb | 6 ++-- spec/functional/func_screen_spec.rb | 25 ++++++------- spec/functional/func_split_screen_spec.rb | 1 + spec/functional/func_table_screen_spec.rb | 24 ++++++++----- .../func_table_screen_updating_spec.rb | 2 ++ spec/functional/func_web_screen_spec.rb | 5 ++- spec/helpers/test_helper.rb | 13 ++++++- spec/unit/screen_helpers_spec.rb | 12 +++---- spec/unit/screen_spec.rb | 2 +- spec/unit/split_screen_open_screen_spec.rb | 6 ++-- spec/unit/tables/table_module_spec.rb | 35 ++++++++++++++----- 14 files changed, 95 insertions(+), 52 deletions(-) diff --git a/Rakefile b/Rakefile index bbc430c..cfad550 100644 --- a/Rakefile +++ b/Rakefile @@ -1,8 +1,8 @@ # -*- coding: utf-8 -*- -unless File.exist?("/Library/RubyMotion2.33/lib") - abort "Couldn't find RubyMotion 2.33. Run `sudo motion update --cache-version=2.33`." +unless File.exist?("/Library/RubyMotion2.34/lib") + abort "Couldn't find RubyMotion 2.34. Run `sudo motion update --cache-version=2.34`." end -$:.unshift("/Library/RubyMotion2.33/lib") +$:.unshift("/Library/RubyMotion2.34/lib") require 'motion/project/template/ios' require 'bundler' Bundler.require(:development) diff --git a/app/test_screens/functional_screen.rb b/app/test_screens/functional_screen.rb index c802cf2..4373cc6 100644 --- a/app/test_screens/functional_screen.rb +++ b/app/test_screens/functional_screen.rb @@ -1,5 +1,6 @@ class FunctionalScreen < PM::Screen attr_accessor :button_was_triggered + attr_accessor :on_back_fired title "Functional" @@ -11,4 +12,8 @@ class FunctionalScreen < PM::Screen def triggered_button self.button_was_triggered = true end + + def on_back + @on_back_fired = true + end end diff --git a/app/test_screens/navigation_screen.rb b/app/test_screens/navigation_screen.rb index 54c92eb..15f7b28 100644 --- a/app/test_screens/navigation_screen.rb +++ b/app/test_screens/navigation_screen.rb @@ -1,9 +1,8 @@ class NavigationScreen < PM::Screen - attr_accessor :on_back_fired + attr_reader :on_back_fired def on_back - self.on_back_fired = true + @on_back_fired = true end - end diff --git a/lib/ProMotion/cocoatouch/navigation_controller.rb b/lib/ProMotion/cocoatouch/navigation_controller.rb index d098450..219d485 100644 --- a/lib/ProMotion/cocoatouch/navigation_controller.rb +++ b/lib/ProMotion/cocoatouch/navigation_controller.rb @@ -2,10 +2,8 @@ module ProMotion class NavigationController < UINavigationController def popViewControllerAnimated(animated) - if self.viewControllers[-2].respond_to? :on_back - self.viewControllers[-2].send(:on_back) - end - super animated + super + self.viewControllers.last.send(:on_back) if self.viewControllers.last.respond_to?(:on_back) end def shouldAutorotate diff --git a/spec/functional/func_screen_spec.rb b/spec/functional/func_screen_spec.rb index 59e1e30..a5aa64f 100644 --- a/spec/functional/func_screen_spec.rb +++ b/spec/functional/func_screen_spec.rb @@ -1,6 +1,8 @@ describe "ProMotion::Screen functionality" do tests PM::Screen + before { UIView.setAnimationDuration 0.01 } + # Override controller to properly instantiate def controller rotate_device to: :portrait, button: :bottom @@ -69,23 +71,22 @@ describe "ProMotion::Screen functionality" do end it "should call the on_back method on the root controller when navigating back" do - @nav_screen = NavigationScreen.new nav_bar: true - @presented_screen = PresentScreen.new - @nav_screen.open @presented_screen - @presented_screen.close - @nav_screen.on_back_fired.should == true + presented_screen = PresentScreen.new + @controller.open presented_screen, animated: false + @controller.navigationController.viewControllers.should == [ @controller, presented_screen ] + presented_screen.close animated: false + @controller.on_back_fired.should == true end it "should call the correct on_back method when nesting screens" do - @base_screen = NavigationScreen.new nav_bar: true - @child_screen = @base_screen.open NavigationScreen.new - @grandchild_screen = @child_screen.open NavigationScreen.new + child_screen = @controller.open NavigationScreen.new, animated: false + grandchild_screen = child_screen.open NavigationScreen.new, animated: false # start closing - @grandchild_screen.close - @child_screen.on_back_fired.should == true - @child_screen.close - @base_screen.on_back_fired.should == true + grandchild_screen.close animated: false + child_screen.on_back_fired.should == true + child_screen.close animated: false + @controller.on_back_fired.should == true end it "should allow opening and closing a modal screen" do diff --git a/spec/functional/func_split_screen_spec.rb b/spec/functional/func_split_screen_spec.rb index f59aab8..0e29562 100644 --- a/spec/functional/func_split_screen_spec.rb +++ b/spec/functional/func_split_screen_spec.rb @@ -10,6 +10,7 @@ describe "Split screen functionality" do end before do + UIView.setAnimationDuration 0.01 rotate_device to: :landscape, button: :right end diff --git a/spec/functional/func_table_screen_spec.rb b/spec/functional/func_table_screen_spec.rb index e267abb..49e0232 100644 --- a/spec/functional/func_table_screen_spec.rb +++ b/spec/functional/func_table_screen_spec.rb @@ -15,9 +15,15 @@ describe "ProMotion::TableScreen functionality" do end def confirmation_class - TestHelper.ios7 ? UITableViewCellDeleteConfirmationButton : UITableViewCellDeleteConfirmationControl + TestHelper.ios7 ? "UITableViewCellDeleteConfirmationButton" : "UITableViewCellDeleteConfirmationControl" end + def table_label_class + TestHelper.ios7 ? "UILabel" : "UITableViewLabel" + end + + before { UIView.setAnimationDuration 0.01 } + after { @table_screen = nil } it "should have a navigation bar" do @@ -31,7 +37,7 @@ describe "ProMotion::TableScreen functionality" do it "should add a new table cell on tap" do tap("Add New Row") - view("Dynamically Added").class.should == UILabel + view("Dynamically Added").class.to_s.should == table_label_class end it "should do nothing when no action specified" do @@ -47,7 +53,7 @@ describe "ProMotion::TableScreen functionality" do it "should delete the specified row from the table view on tap" do table_screen.tableView(table_screen.tableView, numberOfRowsInSection:0).should == 7 tap("Delete the row below") - wait 0.3 do + wait 0.11 do table_screen.tableView(table_screen.tableView, numberOfRowsInSection:0).should == 6 end end @@ -55,7 +61,7 @@ describe "ProMotion::TableScreen functionality" do it "should delete the specified row from the table view on tap with an animation" do table_screen.tableView(table_screen.tableView, numberOfRowsInSection:0).should == 7 tap("Delete the row below with an animation") - wait 0.3 do + wait 0.11 do table_screen.tableView(table_screen.tableView, numberOfRowsInSection:0).should == 6 end end @@ -66,12 +72,12 @@ describe "ProMotion::TableScreen functionality" do table_screen.cell_was_deleted.should != true flick("Just another deletable blank row", :to => :left) - wait 0.25 do + wait 0.11 do # Tap the delete button view('Just another deletable blank row').superview.superview.subviews.each do |subview| - if subview.class == confirmation_class + if subview.class.to_s == confirmation_class tap subview - wait 0.25 do + wait 0.11 do table_screen.tableView(table_screen.tableView, numberOfRowsInSection:0).should == 6 table_screen.cell_was_deleted.should == true end @@ -85,12 +91,12 @@ describe "ProMotion::TableScreen functionality" do table_screen.cell_was_deleted.should != true flick("A non-deletable blank row", :to => :left) - wait 0.25 do + wait 0.11 do # Tap the delete button view('A non-deletable blank row').superview.superview.subviews.each do |subview| if subview.class == confirmation_class tap subview - wait 0.25 do + wait 0.11 do table_screen.tableView(table_screen.tableView, numberOfRowsInSection:0).should == 7 table_screen.cell_was_deleted.should != false end diff --git a/spec/functional/func_table_screen_updating_spec.rb b/spec/functional/func_table_screen_updating_spec.rb index ae650b8..8ddc39a 100644 --- a/spec/functional/func_table_screen_updating_spec.rb +++ b/spec/functional/func_table_screen_updating_spec.rb @@ -1,6 +1,8 @@ describe "ProMotion::TableScreen updating functionality" do tests PM::UpdateTestTableScreen + before { UIView.setAnimationDuration 0.01 } + it 'should update a single row in the table view' do table_screen = UpdateTestTableScreen.new table_screen.make_more_cells diff --git a/spec/functional/func_web_screen_spec.rb b/spec/functional/func_web_screen_spec.rb index f9255aa..8461e92 100644 --- a/spec/functional/func_web_screen_spec.rb +++ b/spec/functional/func_web_screen_spec.rb @@ -1,7 +1,10 @@ describe "ProMotion::TestWebScreen functionality" do extend WebStub::SpecHelpers - before { disable_network_access! } + before do + disable_network_access! + UIView.setAnimationDuration 0.01 + end after { enable_network_access! } def controller diff --git a/spec/helpers/test_helper.rb b/spec/helpers/test_helper.rb index da6a4ea..b9d6133 100644 --- a/spec/helpers/test_helper.rb +++ b/spec/helpers/test_helper.rb @@ -1,6 +1,17 @@ class TestHelper + def self.ios6 + UIDevice.currentDevice.systemVersion.to_f >= 6.0 && + UIDevice.currentDevice.systemVersion.to_f < 7.0 + end + def self.ios7 - UIDevice.currentDevice.systemVersion.to_f >= 7.0 + UIDevice.currentDevice.systemVersion.to_f >= 7.0 && + UIDevice.currentDevice.systemVersion.to_f < 8.0 + end + + def self.ios8 + UIDevice.currentDevice.systemVersion.to_f >= 8.0 && + UIDevice.currentDevice.systemVersion.to_f < 9.0 end end diff --git a/spec/unit/screen_helpers_spec.rb b/spec/unit/screen_helpers_spec.rb index 5188792..3b298d3 100644 --- a/spec/unit/screen_helpers_spec.rb +++ b/spec/unit/screen_helpers_spec.rb @@ -100,13 +100,13 @@ describe "screen helpers" do it "#push_view_controller should use the default navigation controller if not provided" do vcs = @screen.navigationController.viewControllers - @screen.push_view_controller @second_vc + @screen.push_view_controller @second_vc, @screen.navigationController, false @screen.navigationController.viewControllers.count.should == vcs.count + 1 end it "#push_view_controller should use a provided navigation controller" do second_nav_controller = UINavigationController.alloc.initWithRootViewController @screen - @screen.push_view_controller @second_vc, second_nav_controller + @screen.push_view_controller @second_vc, second_nav_controller, false second_nav_controller.viewControllers.count.should == 2 end @@ -200,7 +200,7 @@ describe "screen helpers" do it "should ignore its own navigation controller if current screen has a navigation controller" do basic = BasicScreen.new(nav_bar: true) # creates a dangling nav_bar that will be discarded. - screen = @screen.open basic + screen = @screen.open basic, animated: false screen.should.be.kind_of BasicScreen basic.navigationController.should == @screen.navigationController end @@ -302,13 +302,13 @@ describe "screen helpers" do it "#send_on_return should pass args to the first screen with :root" do first_screen = HomeScreen.new(nav_bar: true) - second_screen = first_screen.open(BasicScreen) - second_screen.open @screen + second_screen = first_screen.open(BasicScreen, animated: false) + second_screen.open @screen, animated: false second_screen.stub!(:on_return) { |args| should.flunk "shouldn't call on_return on second_screen!" } first_screen.mock!(:on_return) { |args| args[:key].should == :value } - @screen.close({ key: :value, to_screen: :root }) + @screen.close key: :value, to_screen: :root, animated: false end end diff --git a/spec/unit/screen_spec.rb b/spec/unit/screen_spec.rb index 14a73bb..143dd30 100644 --- a/spec/unit/screen_spec.rb +++ b/spec/unit/screen_spec.rb @@ -60,7 +60,7 @@ describe "screen properties" do # Issue https://github.com/clearsightstudio/ProMotion/issues/109 it "#should_autorotate should fire when shouldAutorotate fires when in a navigation bar" do parent_screen = BasicScreen.new(nav_bar: true) - parent_screen.open @screen + parent_screen.open @screen, animated: false @screen.mock!(:should_autorotate) { true.should == true } parent_screen.navigationController.shouldAutorotate end diff --git a/spec/unit/split_screen_open_screen_spec.rb b/spec/unit/split_screen_open_screen_spec.rb index e8d33ed..023d67d 100644 --- a/spec/unit/split_screen_open_screen_spec.rb +++ b/spec/unit/split_screen_open_screen_spec.rb @@ -31,14 +31,14 @@ describe "split screen `open` functionality" do end it "should open a new screen in the master view's navigation controller" do - screen = @master_screen.open @detail_screen_2 + screen = @master_screen.open @detail_screen_2, animated: false @split_screen.detail_screen.should == @detail_screen_1 # no change @master_screen.navigationController.topViewController.should == @detail_screen_2 screen.should == @detail_screen_2 end it "should open a new modal screen in the detail view" do - screen = @detail_screen_1.open @detail_screen_2, modal: true + screen = @detail_screen_1.open @detail_screen_2, modal: true, animated: false @split_screen.detail_screen.should == @detail_screen_1 @detail_screen_1.presentedViewController.should == (@detail_screen_2.navigationController || @detail_screen_2) screen.should == @detail_screen_2 @@ -47,7 +47,7 @@ describe "split screen `open` functionality" do it "should not interfere with normal non-split screen navigation" do home = HomeScreen.new(nav_bar: true) child = BasicScreen.new - screen = home.open child, in_detail: true, in_master: true + screen = home.open child, in_detail: true, in_master: true, animated: false home.navigationController.topViewController.should == child screen.should == child end diff --git a/spec/unit/tables/table_module_spec.rb b/spec/unit/tables/table_module_spec.rb index 3f1978e..2c92638 100644 --- a/spec/unit/tables/table_module_spec.rb +++ b/spec/unit/tables/table_module_spec.rb @@ -5,7 +5,8 @@ describe "PM::Table module" do end def custom_cell - @image = UIImage.imageNamed("list") + @image ||= UIImage.imageNamed("list") + @pattern_image_color ||= UIColor.colorWithPatternImage(@image) cell_factory({ title: "Crazy Full Featured Cell", subtitle: "This is way too huge..see note", @@ -30,12 +31,28 @@ describe "PM::Table module" do radius: 15 }, properties: { - masks_to_bounds: true, - background_color: UIColor.colorWithPatternImage(@image) + content_view: { + background_color: @pattern_image_color, + }, + layer: { + masks_to_bounds: true, + }, } }) end + def default_cell_height + return UITableViewAutomaticDimension if TestHelper.ios8 + return 44.0 if TestHelper.ios7 + end + + def default_header_height + # Thanks, Apple. + return -1.0 if TestHelper.ios8 + return 23.0 if TestHelper.ios7 + return 22.0 if TestHelper.ios6 + end + before do @subject = TestTableScreen.new @subject.mock! :table_data do @@ -101,7 +118,7 @@ describe "PM::Table module" do end it "should return the table's cell height if none is given" do - @subject.tableView(@subject.table_view, heightForRowAtIndexPath:@ip).should == 44.0 # Built-in default + @subject.tableView(@subject.table_view, heightForRowAtIndexPath:@ip).should == default_cell_height end it "should allow setting a custom cell height" do @@ -140,11 +157,12 @@ describe "PM::Table module" do it "should set a custom cell background image" do @image.should.not.be.nil - ip = NSIndexPath.indexPathForRow(0, inSection: 3) # Cell 2-1 + ip = NSIndexPath.indexPathForRow(0, inSection: 3) # Cell 4-1 cell = @subject.tableView(@subject.table_view, cellForRowAtIndexPath: ip) cell.should.be.kind_of(UITableViewCell) - cell.backgroundColor.should.be.kind_of(UIColor) - cell.backgroundColor.should == UIColor.colorWithPatternImage(@image) + cell.layer.masksToBounds.should == true + cell.contentView.backgroundColor.should.be.kind_of(UIColor) + cell.contentView.backgroundColor.should == @pattern_image_color end it "should set a custom cell background color" do @@ -162,8 +180,7 @@ describe "PM::Table module" do end it "should use the default section height if none is specified" do - header_height = (UIDevice.currentDevice.systemVersion.to_f >= 7.0) ? 23.0 : 22.0 - @subject.tableView(@subject.table_view, heightForHeaderInSection:4).should == header_height # Built-in default + @subject.tableView(@subject.table_view, heightForHeaderInSection:4).should == default_header_height end it "should use the set title_view_height if one is specified" do From 0845ef897e965ebecd159ada2ddc912a31bef983 Mon Sep 17 00:00:00 2001 From: Jamon Holmgren Date: Sat, 11 Oct 2014 13:01:32 -0700 Subject: [PATCH 6/6] Travis fix --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2410f21..a47df2d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ before_install: - (ruby --version) - sudo chown -R travis ~/Library/RubyMotion - mkdir -p ~/Library/RubyMotion/build - - sudo motion update --cache-version=2.33 + - sudo motion update --cache-version=2.34 gemfile: - Gemfile script: