Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rethink view customization #223

Open
jensljungblad opened this issue Feb 21, 2017 · 8 comments
Open

Rethink view customization #223

jensljungblad opened this issue Feb 21, 2017 · 8 comments

Comments

@jensljungblad
Copy link
Collaborator

jensljungblad commented Feb 21, 2017

Our partial overrides are still slow when you have a lot of records. We've discussed other ways to customize how data is presented. There's presenter objects, and there's what Administrate does with field types.

Currently the way you customize how a resource's data is presented is by overriding column partials, by placing a file in views/articles/columns/_title.html.erb:

<strong><%= resource.title %></strong>

This is implemented using our custom Resolver and by searching for if the partial exists when rendering.

Administrate (https://github.com/thoughtbot/administrate) uses a concept they call fields to do something similar. Field types are specified up front:

ATTRIBUTE_TYPES = {
  id: Field::Integer,
  title: Field::String,
  authors: Field::HasMany,
  created_at: Field::DateTime
}

These can be customized with options when configuring:

ATTRIBUTE_TYPES = {
  title: Field::String.with_options(truncate: 50),
  authors: Field::HasMany.with_options(limit: 5)
}

You can also create partials in views/fields/index/_number.html.erb, views/fields/show/_number.html.erb or views/fields/form/_number.html.erb.

You can create your own fields as well, in app/fields/custom_field.rb:

class ReverseField < Administrate::Field::Base
  def reverse
    data.reverse
  end
end

And in views/fields/index/_reverse.html.erb:

<%= field.reverse %>

One thing we could look into would be having such objects act as presenters, with access to the view context. That way you could customize apperance without overriding any partials. Not quite sure what that would look like.

@juliaramstedt
Copy link

In order to change anything about the way Godmin displays a resource's attributes, partials have to be overridden e.g. views/a_resource/columns/_an_attribute.html.erb. Overriding one of these partials affects the attribute's display on both index.html.erb and show.html.erb. On the partial, you have access to the resource through the variable name "resource" and functions can be applied to it.

Administrate has the with_options function on the Fields objects. These can be used for many simple customisation-scenarios.

Let's exemplify with some Use cases.

Given these resources:

class Author < ApplicationRecord
  has_many :articles
end
class Article < ApplicationRecord
  belongs_to :author
end

Use case: Print single related resource on resource's index

For this scenario, we would like to display the author on articles index view.

Using Godmin:

Add this partial: views/article/columns/_author.html.erb with this content:

<%= resource.author %>

Using Administrate:

In dashboards/article_dashboard.rb, make sure ATTRIBUTE_TYPES contains author: Field::BelongsTo:

  ATTRIBUTE_TYPES = {
    author: Field::BelongsTo,
  }.freeze

and add the following method to dashboards/author_dashboard.rb

  def display_resource(author)
    author.name
  end

Use case: Print multiple related resources on resource's index

For this scenario we would like to display the authors' articles on the author index view.

Using Godmin:

Add this partial: views/authors/columns/_articles.html.erb with this content:

<%= resource.articles.join(", ") %>

Using Administrate:

By doing the same as in the above scenario, we get e.g. "17 articles" on the author index page, but if we would like to print out the article names, we have to create or generate views/admin/authors/_collection.html.erb and customise it. There is a generator for it:

rails generate administrate:views:index Author

Or, alternatively it's possible to override how the has_many field is displayed:

rails generate administrate:views:field has_many

and change views/fields/has_many/_index.html.erb, however that affects all places where a has many field is printed out.

Suggestions based on the above:

  1. Rename the "columns" path to "attributes", since columns bring tables to mind and it's then not apparent that it also changes the way the attribute is displayed on show.html.erb.

  2. Create generators for partials, e.g.

rails generate godmin:views:authors
  1. Can Godmin be able to print out a related resource by default? So that just adding
  attrs_for_index

to article_service.rb is enough? That would be cool.

We could also compare applying a simple function to the attribute.

@Linuus
Copy link
Contributor

Linuus commented Apr 25, 2017

Just a note, both of the Godmin examples above can be done without overriding views. The symbols passed to attrs_for_index are called as methods on the resource so you can define whatever you want there.

attrs_for_index :title, :author_name

Then define the author_name method on your own class. The same goes for the other example. It's really only when you need more complex UI stuff that you need to override the partial.

@Linuus
Copy link
Contributor

Linuus commented Apr 25, 2017

Also, doesn't it already work with single association if it defines a to_s method?

@jensljungblad
Copy link
Collaborator Author

Good point, forgot that it calls the method that way. Perhaps we should put that in the readme? Is that good enough then or do we have other reasons for introducing say a presenter?

I too think renaming the column folders would be good. Either fields or attributes. Not sure we really need the ability to customize based on index or show, like Administrate. Seems rare and it's possible now by checking the action param in the partial.

@jensljungblad
Copy link
Collaborator Author

I think we removed support for associations in columns, and linking to the associated record, but I could be wrong. Would be good to have though, as well as support for has many.

@Linuus
Copy link
Contributor

Linuus commented Apr 25, 2017

Yes we removed the automatic linking of associations but they should print ok though? :)

@jensljungblad
Copy link
Collaborator Author

I guess one reason for manually specifying types would be performance? Our previous solution for checking what type a certain field was involved reflection, I think. It's what we do in our form builder as well. Manually specifying types, while a bit cumbersome perhaps, would mean we could skip reflection when generating the forms, and for when we want to render an attribute in the table or on the show page, based on what type it is.

@jensljungblad
Copy link
Collaborator Author

jensljungblad commented Nov 22, 2017

If we want to get away from partial overriding because of #221, this could be one way.

Case 1. You only want to tweak the attribute value slightly

We could do this like we do now, by delegating calls to the model:

class ArticleDashboard
  ATTRIBUTE_TYPES = {
    title: Godmin::Field::String
    title_with_author: Godmin::Field::String
  }
  LIST_PAGE_ATTRIBUTES = %i[title_with_author]
  FORM_PAGE_ATTRIBUTES = %i[title]
end

class Article
  def title_with_author
    "#{title} by #{author}"
  end
end

Or by generating a presenter:

class ArticleDashboard
  ATTRIBUTE_TYPES = {
    title: Godmin::Field::String
    title_with_author: Godmin::Field::String
  }
  LIST_PAGE_ATTRIBUTES = %i[title_with_author]
  FORM_PAGE_ATTRIBUTES = %i[title]
end

class ArticlePresenter
  def title_with_author
    "#{title} by #{author}"
  end
end

Something like this is much faster than doing a partial lookup, so makes sense for smaller changes. Perhaps it is slightly confusing if we introduce ATTRIBUTE_TYPES though, since in the above examples, it wouldn't work to add title_with_author to FORM_PAGE_ATTRIBUTES for instance. Typically we might want different presentation of the same attribute on index and show compared to new and edit. If we are okay with the concept of attribute presentation only really showing up on index and show, never in forms, I guess we could also do something like:

class ArticleDashboard
  ATTRIBUTE_TYPES = {
    title: Field::Title
  }
  LIST_PAGE_ATTRIBUTES = %i[title]
  FORM_PAGE_ATTRIBUTES = %i[title]

  class Field::Title < Godmin::Field::String
    def value
      "#{record.title} by #{record.author}"
    end
  end
end

Case 2: You want to change the markup for the attribute value

Perhaps something similar to the above?

class ArticleDashboard
  ATTRIBUTE_TYPES = {
    title: Field::Title
  }
  LIST_PAGE_ATTRIBUTES = %i[title]
  FORM_PAGE_ATTRIBUTES = %i[title]

  class Field::Title < Godmin::Field::String; end
end

The Field class would have a to_partial_path that would look for the partial first in app/views/article_dashboard/field/_title.html.erb then in app/views/godmin/field/_string.html.erb.

Shared fields could be placed outside of a dashboard, and partials placed in app/views/field/_shared_field.html.erb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Godmin 2.0
Discussions
Development

No branches or pull requests

3 participants