jamonholmgren / ProMotion

ProMotion is a RubyMotion gem that makes iPhone development less like Objective-C and more like Ruby.
MIT License
1.26k stars 147 forks source link

Add scopes to searchable module #509

Open markrickert opened 10 years ago

markrickert commented 10 years ago

There needs to be a way to set a search scope so that if you set scopes on cells, the scope buttons appear under the search bar. Cells should be able to belong to multiple scopes.

jamonholmgren commented 10 years ago

Can you elaborate? I'm not following entirely.

markrickert commented 10 years ago

Like this: m83gn

I'll be working on adding this soon since I need it for one of my apps :)

markrickert commented 10 years ago

My idea is that if you define cells with this additional parameter:

{
  # ...
  scope: :one
},
{
  # ...
  scope: [:one, :two]
},
{
  # ...
  scope: :two
}

It would automatically set the search_bar.showsScopeBar = true and use the symbols to create the two scopes: "One" and "Two". When tapping on the scope buttons it limits the search criteria to those cells so when you tap "One", cells 0,1 show up, when you tap "Two", cells 1,2 show up.

We might want to have a mechanism to turn on :all so that a user doesn't have to specify :all in each cell.

Thoughts on this design pattern?

jamonholmgren commented 10 years ago

I think that would be cool; what I'm not sure about is whether it makes sense to include this functionality in PM core. How much code would we be introducing, do you think?

markrickert commented 10 years ago

Probably not more than a few methods. The scoping is a native API to the UISearchBar component.

jamonholmgren commented 10 years ago

That's great. I'd be fine with this. I think :all would be default and we'd turn on scoping if needed.

class MyTable < PM::TableScreen
  searchable placeholder: "Search", scoped: true
  def table_data
    [{
      cells: [{
        scope: :one,
        # ...
      }]
    ]}
  end
end
jamonholmgren commented 10 years ago

I think the scope should probably be a string, actually, instead of a symbol. Makes more sense in this context.

markrickert commented 10 years ago

Yeah, I agree that it should probably be a string, but since strings are created in memory each time and symbols are reused in memory, I thought for larger datasets that symbols would be more appropriate, then titleize them. You've got to set the scope button titles when you initialize the UISearchBar and they can't change.

I can make this its own gem, but i think that it fits nicely into the functionality of ProMotion core. But just becuase I need it and think it's a good idea, doesn't mean it should be in core :) I have no problem just extending the search functionality through a gem: ProMotion-searchable-scope or something like that.

jamonholmgren commented 10 years ago

No, I think it's cool in core, since it's native functionality and pretty slim. And if we include it in searchable, that whole module could be split into its own add-on gem eventually if we decided it was too big for core (probably won't ever happen).

markrickert commented 10 years ago

Cool, well I'll get started on adding this functionality tonight and we'll leave it in the searchable module for now.

jamonholmgren commented 10 years ago

Regarding symbols, I think maybe we just allow either (and .to_s them), since some use cases may need irregular capitalization (that titleize wouldn't work for).

We'll probably need to specify the scopes in the searchable method call, since table_data may be empty when you first initialize the screen.

searchable scoped: [ "First", "Second", "Third" ]
markrickert commented 10 years ago

Yeah, good call. I'll have something to look at later tonight probably.

jamonholmgren commented 10 years ago

:+1:

markrickert commented 10 years ago

here's the initial work done: https://dl.dropboxusercontent.com/u/12954/search_scopes.mov

Working on getting the ":all" scope implemented now

markrickert commented 10 years ago

Initial work is in this branch: https://github.com/clearsightstudio/ProMotion/tree/search_scopes

Change the app delegate to open TableScreenSearchable.new(nav_bar: true) to see it in action!

markrickert commented 10 years ago

OK, done for the evening. @jamonholmgren, have a look at that branch and let me know what you think so far. Everything is working, including indicating a custom string that you want for "All" to search.

jamonholmgren commented 10 years ago

Nice work, @markrickert! I'm holding off on merging until we get 2.0.0 out the door, but then we'll have a lot of cool new features for 2.1 already. :)

2.0 should be released soon. Just need to find about an hour to look it over again and then push it out with a blog post.

markrickert commented 10 years ago

Yeah. Understood. Any comments in the way I implemented it or the :all feature where you can specify your own button title?

I'll continue to use my "franken-fork" for all these features I need in prod. lol

jamonholmgren commented 10 years ago

Haha. I'll look at it this weekend.