Skip to content
This repository was archived by the owner on Nov 19, 2025. It is now read-only.

Conversation

@forrestgrant
Copy link
Contributor

No description provided.

@markrickert
Copy link
Contributor

Nice! Really good PR. I'm in favor of merging this. 👍

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple nitpicky things:

  1. We should explicitly check whether it's a String, UIView, or UIImage.
  2. String = what you have in line 60
  3. UIView = self.navigationItem.titleView = self.class.send(:get_title)
  4. UIImage = what you have in line 62
  5. Else = PM.logger.warn("title expects string, UIView, or UIImage, but #{self.class.send(:get_title).class.to_s} given."

@forrestgrant
Copy link
Contributor Author

All set, @jamonholmgren.

jamonholmgren added a commit that referenced this pull request Feb 5, 2014
Allowing screen title to be UIImageView
@jamonholmgren jamonholmgren merged commit 7f6c63a into jamonholmgren:edge Feb 5, 2014
@jamonholmgren
Copy link
Owner

Thanks @forrestgrant !!

@markrickert
Copy link
Contributor

Wiki? 😁

@forrestgrant
Copy link
Contributor Author

I'll add it to the Wiki, @markrickert.

@markrickert
Copy link
Contributor

💖 👍 💖

@forrestgrant
Copy link
Contributor Author

So it looks like the title=(t) method doesn't work with UIView|UIImageView, because resolve_title is called in on_create.

Should we move it here:

# /lib/ProMotion/cocoatouch/view_controller.rb
module ProMotion
  class ViewController < UIViewController
    # ...
    def loadView
      super
      self.send(:on_load) if self.respond_to?(:on_load)
      self.send(:resolve_title) if self.respond_to?(:resolve_title)
    end
    # ...
  end
end

@jamonholmgren
Copy link
Owner

Ooh, didn't catch that. :/

Another question is if it works to change the title in a method:

class HomeScreen < PM::Screen
  title UIImage.imageNamed("loading")

  def on_load
    self.title = UIView.imageNamed("loading_complete")
  end

@forrestgrant
Copy link
Contributor Author

I didn't catch it either, until I started writing more robust tests.

In any case, I think we have 3 options:

  1. Rollback the feature
  2. Only support UIImage|UIView in on_create (current)
  3. Add resolve_title to title=(t)

Thoughts?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants