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

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

Closed
eddietejeda opened this issue Apr 27, 2016 · 5 comments
Closed

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

eddietejeda opened this issue Apr 27, 2016 · 5 comments

Comments

@eddietejeda
Copy link
Contributor

eddietejeda commented Apr 27, 2016

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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants