RFC: View hooks beyond Deface

[tvdeyen]

The problem

Extensions and stores want to make additions (or even changes) to the admin views. As of yet, the defacto standard for this is Deface. Deface brings a lot of other problems - first and foremost it hides view changes into places where they are hard to track down or introduces "where does this checkbox coming from?" problems. Also it is barely known outside the Spree/Solidus ecosystem. Additionally we need to maintain HTML hooks (data-hook) that exist solely for Deface to work properly. That makes admin views refactoring very hard.

Proposal

I am proposing an idea that originally came up from @mamhoff while working with him on a client store that makes lots of changes to the Solidus admin. We removed Deface from that store a while ago and introduced another way to make admin view changes maintainable:

Rails partials

By splitting up the admin views into smaller chunks we make it easier for us to keep our admin maintainable during Solidus updates.

This is how it works

The Admin Orders table for example.

We split the admin/orders/index.html.erb template into several partials:

1. The filter fields

...
<% content_for :table_filter do %>
  <div data-hook="admin_orders_index_search">
    <%= search_form_for [:admin, @search] do |f| %>
      <%= render 'filter_fields', f: f %>
      ...
    <% end %>
  </div>
<% end %>
...

2. The table

...
<% if @orders.any? %>
  <%= render 'table', orders: @orders, ransack_search: @search, show_only_completed: @show_only_completed %>
<% else %>
  ...
<% end %>
...

3. The table row

<table class="index" id="listing_orders" data-hook>
  <colgroup>
    ...
  </colgroup>
  <thead>
    ...
  </thead>
  <tbody>
    <%= render collection: orders, partial: 'spree/admin/orders/order' %>
  </tbody>
</table>

Summary

For index views I propose implementing a pattern like:

admin/resource/index.html.erb
  - admin/resource/_filter_fields.html.erb
  - admin/resource/_table.html.erb
    - admin/resource/_resource.html.erb

For edit views I propose implementing a pattern like:

admin/resource/edit.html.erb
  - admin/resource/_form.html.erb
    - admin/resource/_form_additions.html.erb
  - admin/resource/_side_bar.html.erb
    - admin/resource/_side_bar_additions.html.erb

I think adopting this into core will make future changes to the admin easier for us and extension maintainers as well as stores. Sure, this is a lot of work and there will be many cases where need to find a good solution for, but still I think it is worth it.

[elia]

Super excited about replacing deface with smaller replaceable pieces!!! 😍

The one consideration/downside that immediately comes to mind is that partials are really slow 🐌 (for each one of them has to build an instance of ActionView with its context, included modules, etc.).

So I would use the technique you proposed, but with something like ActionView::Component (https://github.com/rails/rails/pull/36388) that will be super fast ⚡ and can be extended with "decorators" as we do for any other customization. 👷‍♂️

On that topic I think we don't even need to wait for ActionView::Component to be officially part of Rails to use it, instead a compatible-enough Solidus::Component can be introduced and later switched 🔧 to the Rails implementation when the time comes (i.e. we drop support for Rails 5).

[kennyadsl]

I like the idea and I'm happy to start a conversation about this but I have some concerns, which I think are some of the reasons why we are still relying on Deface:

1. Backward compatibility with existing Deface hooks

A Deface hook at the moment has a virtual_path parameter that describes where to find the view that we want to manipulate. If we change the structure of a view splitting it into multiple partials, Deface will not be able to find where existing hooks are defined. If someone wants to keep using Deface, this will likely require some manual work in both extensions and store to reconnect Deface hooks with its new virtual path. On the other hand, we want to make this change to remove Deface, so it will be developers responsibility to make this work if they want to continue using it. Still something worth a major release in my opinion.

2. Multiple overrides in the same partial from different extensions

The more we split views, the less probable is that two extensions override the same partials but this is still a thing to consider. I think the majority of place where this happens is into shared spaces like menus or tabs, but for those, I think we need to provide an interface in core to easily change them via code configuration and not via Deface (something similar to what we have done here).

3. How to find partials to override

I think this approach will introduce a new problem, similar to the

"where does this checkbox coming from?"

problem. When we need to override a partial to add our custom stuff, we need to understand which version of the partial we are currently using. This means scanning all extensions and understand where the current version comes from, taking into account gems load order (Gemfile sorting?).

---

An alternative approach that I was thinking about lately is:

• encourage extensions developers to avoid overriding views. A lot of features can be developed by creating a new page, linked in the sidebar or tabs of the resource we are changing. This will lead to add views only for new pages, and less overrides of the existing views
• if an override is needed, provide partials that users can manually require into their code instead:
  • provide instruction to install partial into extension point partials (like the ones you proposed admin/resource/_form_additions.html.erb)
  • use deface only in test environment, so we can safely test partials that will be manually included. We started making something similar into solidus_customer_images

What do you think?

[tvdeyen]

Thanks @elia and @kennyadsl !

On that topic I think we don't even need to wait for ActionView::Component to be officially part of Rails to use it, instead a compatible-enough Solidus::Component can be introduced and later switched wrench to the Rails implementation when the time comes (i.e. we drop support for Rails 5).

I very much like that Solidus::Component idea :rocket:

On the other hand, we want to make this change to remove Deface, so it will be developers responsibility to make this work if they want to continue using it. Still something worth a major release in my opinion.

Yes, a mayor version for sure :+1:

I think we need to provide an interface in core to easily change them via code configuration and not via Deface.

Absolutely :100:

When we need to override a partial to add our custom stuff, we need to understand which version of the partial we are currently using. This means scanning all extensions and understand where the current version comes from, taking into account gems load order (Gemfile sorting?).

This should be easier to solve if we use code (Solidus::Component and Spree::BackendConfiguration::MenuItem), but still a tough thing to solve, sure.

encourage extensions developers to avoid overriding views. A lot of features can be developed by creating a new page, linked in the sidebar or tabs of the resource we are changing. This will lead to add views only for new pages, and less overrides of the existing views

Yes, and clients as well.

Anyhow, I still think we need to start working on changing the admin into a more maintainable and likewise customizable way.