administrate icon indicating copy to clipboard operation
administrate copied to clipboard

Add Field::CollectionSelect

Open sedubois opened this issue 5 years ago • 6 comments

This is in answer to https://github.com/thoughtbot/administrate/issues/1535#issuecomment-608354608.

TODO:

  • [ ] test

sedubois avatar Apr 05 '20 20:04 sedubois

Just had a cursory look. I'm not sure about Selectize being referred to directly on the Ruby code. Although it helps remove some duplication, I think it tightens the dependency on this specific solution, where users may prefer others in the future.

This is only a half-formed thought at the moment, but I feel that a more flexible way for users to enable/disable Selectize would be to create custom fields, derived from the existing ones, which override the partials and set different class names. Otherwise, we create a solution where it's more difficult to opt out of Selectize.

Does this make sense?

pablobm avatar Apr 10 '20 08:04 pablobm

@pablobm understood about the naming. How about using another name, e.g.

def self.advanced_select_form?
  options.fetch(:advanced_select_form, true)
end

As for using a partial, I felt that passing an option to the field might in general be easier. In particular, it would enable inheritance, e.g. a field extending, say, Field::BelongsTo would automatically inherit the default value from its parent. Unless I'm overlooking something with partials?

sedubois avatar Apr 10 '20 09:04 sedubois

@pablobm could you also try to run the branch to see why I get the error uninitialized constant Administrate::Field::CollectionSelect? It's probably something silly, I'm just a bit new to this.

sedubois avatar Apr 13 '20 11:04 sedubois

@sedubois - I'm still not convinced about that option. The first thing is that I don't like adding that method to Field::Base, as it encumbers its interface with detail that is only relevant for some subclasses.

Also at a more "idealistic" level, I like how currently the forms are JS-enhanced without the Ruby playing any part. It feels to me more extensible, although at this point I admit that I'm working more with gut feel and platonic ideals of progressive enhancement.

pablobm avatar Apr 18 '20 10:04 pablobm

@sedubois - To fix that error, add:

### /lib/administrate/base_dashboard.rb
require "administrate/field/collection_select"

Admittedly, that's probably a less clear part of creating a new field type :-/

pablobm avatar Apr 18 '20 10:04 pablobm

And by all means: please add a test. A field in the example app would be ideal, as it also acts as an example and a quick way to play with it. Thank you!

pablobm avatar Apr 18 '20 10:04 pablobm

Closing due to lack of activity. Also I'm not sure this adds something that cannot be done with Field::Select? Another thing is that doing collection: Customer.all could backfire quickly as it would be resolved on boot and not be updated as the table changes. Instead passing collection: ->{ Customer.all } to Field::Select would be safer.

pablobm avatar Apr 18 '23 09:04 pablobm