Skip to content

Selects in forms#7

Open
leizzer wants to merge 39 commits intotomas:masterfrom
leizzer:master
Open

Selects in forms#7
leizzer wants to merge 39 commits intotomas:masterfrom
leizzer:master

Conversation

@leizzer
Copy link

@leizzer leizzer commented Aug 9, 2012

Hello,

Your work saved me time.

I need some features, so I used the time you save me for add, in this case, a select (drop down) in forms where the model belongs to other.

I only use datamapper, so I changed the datamapper Bowtie::ClassMethods#relation_keys_include? method for something that I can use in the view. I couldn't run automated tests, I didn't see any in the gem :(

If you don't like my work it is OK, but I would really like to see a feature like this.

Thank you for your work.

@tomas
Copy link
Owner

tomas commented Aug 9, 2012

Hi @leizzer, glad to hear Bowtie was useful to you. I just checked your code and it looks fine, but I have a problem with just one thing: the model.all.map call when building the <select> combobox.

That means that if there 1M records you will not only fetch all those 1M records, but you'll also instantiate each one of them meaning the app ill start eating RAM as hell.

In other words, I think the idea is great, I'm just not sure about the implementation. Perhaps using an autocomplete field would work better, in fact, we could use it to show suggestions not only for _id fields but for any field (e.g. when modifying the state field of a record it could get a list of all available states in all records in the DB).

What do you think?

@leizzer
Copy link
Author

leizzer commented Aug 10, 2012

Hi.

You are right about the model.all.map, but I couldn't figure out another way because the method (to_option_select) is called but it isn't a model property.

I think that an autocomplete is a good idea if it can solve the problem of loading so many instances. It all depends on what kind of autocomplete, it's more common that the parent model only have a bunch of entries (in my case around 10) and a drop down is useful for the user to select what they want without knowing the entries that already exist. In other words a mix of autocomplete and dropdown could be a good approach. (I don't remember how rails admin does these things)

Talking about using this for other things more than the _id fields: yes, it's good too. It can load options for Enum properties (in datamapper: peroperty :sex, Enum["f", "m"] ) I think this should be easy to load.

I will be extending bowtie as we need. Some feature that I'm thinking about are: exclude columns in index tables ( e.g. I want hide the password column) and friendly labels (e.g. Change uuid label for Membership Number)(maybe we could implement I18n here)...

I think railsamdmin use a config file for extra tuning, but I really like how I can add bowtie to my project without work..

Thanks for your time.

@tomas
Copy link
Owner

tomas commented Aug 17, 2012

A select box for enum fields (or the ones that have an explicit 'validates inclusion in') sounds great, but I'm still not sure what's the best strategy for association ID's. Does Rails Admin solve this in a specific way?

@leizzer
Copy link
Author

leizzer commented Aug 21, 2012

Hi,

I found that Rails Admin uses a lot of js/ajax.

Rails Admin do something similar, but it sets a limit. (https://github.com/sferik/rails_admin/tree/master/lib/rails_admin/config/fields/association.rb:43).

They don't use an html select, they create something similar with js (jquery ui-autocomplete). And I don't know what they do with the rest of the registers. (http://rails-admin-tb.herokuapp.com/admin/team/58/edit).

@tomas
Copy link
Owner

tomas commented Aug 22, 2012

Looks like that's the only way! I guess we could use the Ajax version of the chosen library for this. It would look much cooler too. :)

@leizzer
Copy link
Author

leizzer commented Aug 22, 2012

It looks nice. How do you want to proceed? Tomorrow if I have time I could try to add this.

Edit: human error with the cellphone

@leizzer leizzer closed this Aug 22, 2012
@leizzer leizzer reopened this Aug 22, 2012
@leizzer
Copy link
Author

leizzer commented Aug 22, 2012

Oh man! I closed it too! I'll never use the cellphone for comment in github again.

@tomas
Copy link
Owner

tomas commented Aug 22, 2012

Haha, no problem. Let me know if you have any questions.

@leizzer
Copy link
Author

leizzer commented Aug 22, 2012

I added the no-ajax version of chosen.js.

I use no-ajax because my first step was make it work. Now I see that the ajax-chosen is an other plugin. I don't know how to implement a "selected" option in ajax-chosen. Maybe you have an idea but I don't want to lose the selected option on Edit.

@tomas
Copy link
Owner

tomas commented Aug 23, 2012

Awesome! I'll take a deeper look at the code when I have a minute. :)

@leizzer
Copy link
Author

leizzer commented Sep 19, 2012

Hi, change a lot in my "config" branch and I need to move on merging it to master.

I will wait some days because if I do the merge the commits in "config" will be added to the pull request.

@tomas
Copy link
Owner

tomas commented Sep 21, 2012

Hey Leizzer,

Sorry for the delay, I just got back from a trip. I just pulled your code and it looks that there are still some bolts missing. At least the mongomapper adapter is missing the adapter_fields_registry and when trying to load the admin panel I get a nasty error:

ArgumentError at /admin/albums -- wrong number of arguments (2 for 1)

[line 44] ../lib/bowtie/helpers.rb in const_defined?
      if ::Bowtie::Models::Extensions.const_defined?(model.to_s, false) && !model.include?(::Bowtie::Models::Extensions.const_get(model.to_s, false))

Besides that, everything looks good. Though I wasn't able to test it thoroughly, it seems like the customization options you added are entirely optional which is good (this gem is meant to work out of the box). Personally I've been wanting to add the ability of specifying custom methods to call on an object directly through the panel, and it looks like it could fit nicely in your Bowtie::Config class.

Just to give you a rough idea of what I have in mind:

class Author
  many :posts
  key :published_posts, Integer, :default => 0
end

class Post
  belongs_to :author
  belongs_to :blog

  def publish
    update_attribute(:state, :published)
    author.increment(:published_posts => 1)
    blog.refresh_sitemap # or whatever
  end

  def unpublish
     # like the one above but all the way around
  end

end

##########

module Bowtie::Models::Extensions
 module User
   def bowtie_methods
     %w(publish unpublish)
   end
 end
end

@tomas
Copy link
Owner

tomas commented Sep 21, 2012

By the way, looking deeper at the customization syntax I think it could be simplified a bit for readability. What do you think of using something like this?

Bowtie.extend do

  model :users do
    option_text lambda { |u| "#{u.name}" }
    include_fields :avatar
    exclude_fields :encrypted_password
  end

  model :posts do
    option_text lambda { |p| "#{p.title} by #{p.author.name}" }
    show_actions :publish, :unpublish
  end

end

@leizzer
Copy link
Author

leizzer commented Sep 26, 2012

Hello,
That error you mention is on my Master branch?

In the branch "config" I didn't paid to much attention to mongoid because I was on a hurry.

Some days ago I see that the gemspec generate errors on my host/server because of the "á" on your name. I added the utf-8 specified on the top, but it still failing. That have to be removed, I think. (I know that the OS can be configured, my computer is configured but it will be simpler).

Tell me if there are something that are stopping you to accept this pull request.

I will ask for merge my config branch in other pull request after take care of mongoid.

About the DSL you are talking about. It looks nice. But like I said, I have no time :(
I like that DSL, because it looks simple and I don like to learn a complex DSL for this.

@tomas
Copy link
Owner

tomas commented Sep 26, 2012

Yes that error is on your branch, and it's not Mongoid what I'm using, it's MongoMapper. Hope that you can give it a bit of time so we can get this merged -- unfortunately it can't be done as it is. :(

@leizzer
Copy link
Author

leizzer commented Sep 27, 2012

I spend a time today checking the error you commented.

"field_registry" and "Extensions" are in the branch "config"; here we are talking only about the "master" branch. Where the changes only add the dropdown.

Test the master branch. I'll make an other pull request for "config" after this one.

Using "git://github.com/leizzer/bowtie.git" (branch Master) in a new project I have no problem.

@tomas
Copy link
Owner

tomas commented Sep 28, 2012

Awesome, let me know when your pull is up so I can merge again and try it out.

@leizzer
Copy link
Author

leizzer commented Oct 3, 2012

Ok, I merged 'Config' into 'Master'.

I didn't have any error in my project using DataMapper, could you try it?.

I tested it with and without using Bowtie Extensions in my app.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants