mrbrdo / spree_mobility

BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Why RTE is used only for some text area fields? #5

Closed Kulgar closed 2 years ago

Kulgar commented 2 years ago

Hello again,

I was wondering, why do you activate rich text editor only for some fields?

https://github.com/mrbrdo/spree_mobility/blob/0a0ffbb6e453dd74ef563143d842d77f8515f480/app/views/spree/admin/translations/_form_fields.html.erb#L17-L26

Wouldn't it be better to activate it for all text areas?

In my app, I will need to activate it, as I added some other translated fields (through a custom decorator), to add some more translatable content to products

mrbrdo commented 2 years ago

I added those which are RTE in Spree itself (without translations), which I think is only description? Which other stock fields would benefit from a RTE?

Kulgar commented 2 years ago

None in the basic Spree app, but I extended Spree Product model with a decorator to add some more "text" fields for my app. And I was surprised not to see the rich text editor for them in the translations tab (and I need it for these custom fields ^^).

This is what I did so far:

            <% field_type = @resource.class.translation_class.columns_hash[attr.to_s].type %>
            <% if @resource.kind_of?(Spree::Product) && field_type == :text %>
              <%= g.text_area attr, { rows: "30", class: "form-control #{"spree-rte" if SpreeMobility.product_wysiwyg_editor_enabled? }" } %>
            <% elsif @resource.kind_of?(Spree::Taxon) && field_type == :text %>
              <%= g.text_area attr, { rows: "30", class: "form-control #{"spree-rte" if SpreeMobility.taxon_wysiwyg_editor_enabled? }" } %>
            <% elsif field_type == :text %>
              <%= g.text_area attr, { rows: "30", class: "form-control #{"spree-rte" if SpreeMobility.product_wysiwyg_editor_enabled? || SpreeMobility.taxon_wysiwyg_editor_enabled? }" } %>
            <% else %>
              <%= g.text_field attr, class: 'form-control' %>
            <% end %>

But maybe we could think about a custom configuration to enable rich text editor for other text area fields? I don't know... what do you think?

mrbrdo commented 2 years ago

The description is RTE in spree_backend itself: https://github.com/spree/spree_backend/blob/main/app/views/spree/admin/products/_form.html.erb#L24

So this is why I made description RTE in the translation form also. I don't think anything more belongs in the core functionality of this gem. The only real solution would be for Spree itself to have a configuration to assign which fields of which models should have an RTE. There are other reasons to use a text field besides for RTE.

I think the best way to go in your case is to use Deface to override this part of the _form_fields template, something like:

Deface::Override.new(
  virtual_path:  'spree/admin/translations/_form_fields',
  replace: 'div.panel-body',
  text:           <<-HTML
                    <div class="panel-body">...</div>
                  HTML
)
Kulgar commented 2 years ago

Thanks @mrbrdo I'll try that