vhochstein / active_scaffold

Rails 4 Version of activescaffold supporting jquery
MIT License
156 stars 34 forks source link

List override behavior #81

Closed clyfe closed 13 years ago

clyfe commented 13 years ago

Because Rails3 includes "all helpers all the time" by default, we should call the list override helpers #{model}_#{column}_column instead of #{column}_column. The official branch already did this.

https://github.com/activescaffold/active_scaffold/wiki/Field-Overrides

vhochstein commented 13 years ago

I think model might be suboptimal cause there might be more than one controller for a model. therefore controller might be a better prefix. However, what happens if you would like to have a name colum form override for all your controllers...?

It seems that the rails guys itself are quite unsure about the best way to deal with the helpers, but they voted for this system that all helpers are shared in all controllers as a default. Which means that you have to deal with it inside of your helper and branch according to the current controller.

Bottomline I m quite unsure about the right way to go... and I m not really convinced of the official branch way of dealing with it. Basically, I would suggest to either revert that helper :all in your controller, or branch in your method according to controller_name

clyfe commented 13 years ago

I have branched the method in the view with an if column.name == :my_column_name

From the philosophical point of view the helpers are meant to code in "generic" functionality and I mostly agree with the "include all helpers". As such the customization is more suited in a view. But then again, list cells are such a small piece of functionality (<td>) that doing a view just for them seems an overkill. I think probably that's why the team made them helpers.

I think a good approach would be similar to the following:

  1. #{model}_#{column_name}_column to override the column in all scaffold of this model, like the official branch. Mostly, different scaffolds should have the same requirements for a certain column.
  2. #{module/controller}_#{column_name}_column in case custom per-controller behavior is needed, and this should have precedence over [1]
vhochstein commented 13 years ago

To be honestI do not see much of an improvement compared to just doing:

{column_name}_column

inside you may do if record.is_a? or controller_path ==...

or whatever you like. You do not have to think about which override is more appropriate you ve got just one column override and inside you can do whatever you want.

Just my two cents...

clyfe commented 13 years ago

Indeed, you are right. Anyway, I think prefixing the helper with the model name is more appropriate considering helper :all, even tough is suboptimal.