thoughtbot / administrate

A Rails engine that helps you put together a super-flexible admin dashboard.
http://administrate-demo.herokuapp.com
MIT License
5.88k stars 1.11k forks source link

I created Administrate::Field::JSON and looking for feedback. #566

Closed eddietejeda closed 8 years ago

eddietejeda commented 8 years ago

I created Administrate::Field::JSON and looking for feedback. I used Administrate:Field:Image created by @graysonwright as a guide.

https://github.com/eddietejeda/administrate-field-json

I use JSONEditor to view and edit the JSON. I found this library be the most complete and flexible of all the JSON editor libraries out there. It might need some styling edits to make it fit in with Administrate's look.

I've been using Administrate for few months now and testing Field::JSON for a couple days. I have not released it to Rubygems yet and would appreciate feedback before doing so.

In creating this plugin, I encountered a few problems with the plugin architecture. I can file these as separate issues, but to quickly summarize:

1) Adding assets to your plugin is tricky. application(js/css) are not automatically picked up by the asset pipeline. In Greyson's sample, he suggests that we edit the Administrate template (https://github.com/graysonwright/administrate-field-nested_has_many). I did not want to do that, so I added the plugin path to the asset pipeline and gave application(js/css) a different name. See example here: https://github.com/eddietejeda/administrate-field-json/blob/master/lib/administrate/field/json.rb. I think that's a better long term solution since it encapsulates the components within the plugin.

2) It was unclear how I can inject my own js into =yield :javascript at a page level, not field level, so I have to flush =content_for to prevent printing out duplicates. https://github.com/eddietejeda/administrate-field-json/blob/master/app/views/fields/json/_form.html.erb Let me know if there is a better way to do this.

3) There was no =content_for :stylesheet, so I am currently using the =content_for :javascript. I propose a simple edit to support =content_for :stylesheet. Here's how that would look like: https://github.com/thoughtbot/administrate/compare/master...eddietejeda:content_for_stylesheet?expand=1

4) For the _show.erb partial, I just display the JSON as within <pre> tags. I could use JSONEditor and mark it as readonly. Would like to hear preferences.

Let me know if Field::JSON works for you all and which of these other observations I should file as issues/pull requests.

Thanks.

c-lliope commented 8 years ago

@eddietejeda, thanks for the PR! I really like a lot of your ideas, and think they would really improve the API for creating plugins. Here are some of my thoughts:

  1. I agree that adding assets is tricky, and I want to improve the process. Your solution of adding a new file to the asset pipeline is a great start.
  2. For loading assets at the page level instead of field leve, it would be great if the default app/views/administrate/application/_javascript.html.erb were able to find and import assets from plugins. I'll start working on a proposal for that.
  3. I really like the content_for :stylesheet idea. I'll merge that in in a minute.
  4. Can you post screenshots of what it looks like with <pre> vs with JSONEditor?
c-lliope commented 8 years ago

@eddietejeda, I just created #573, which should solve the problem of importing assets at the page level.

You can see what it looks like in https://github.com/graysonwright/administrate-field-nested_has_many/pull/2/files.

In your plugin, I think you could get it to work with:

require "administrate/engine"
require "administrate/field/base"
require "rails"

module Administrate
  module Field
    class JSON < Administrate::Field::Base
      VERSION = "0.0.1"

      class Engine < ::Rails::Engine
        Administrate::Engine.add_javascript "administrate-field-json"
        Administrate::Engine.add_stylesheet "administrate-field-json"

        engine_root = self.root

        isolate_namespace Administrate
        config.to_prepare do
          Rails.application.config.assets.paths << engine_root.join("vendor", "assets", "images")
        end
      end
    end
  end
end
c-lliope commented 8 years ago

Let me know what you think, or if you'd prefer a different API. If I don't hear back, I'll merge this in after a few days.

rikkipitt commented 8 years ago

@graysonwright Interesting updates, these are related to #123 and #187. Once these updates have been finalised, I'd be keen to get them in my ckeditor field to make things a bit cleaner! https://github.com/jemcode/administrate-field-ckeditor

c-lliope commented 8 years ago

@rikkipitt and @eddietejeda – I merged in the update. You should both be able to use the new JS syntax if you depend on administrate ~> 0.2.1.

Let me know how it goes!

@eddietejeda – I'm going to close this issue, because it looks like we've addressed most of your questions. Feel free to re-open it if we missed anything.