jekyll-admin icon indicating copy to clipboard operation
jekyll-admin copied to clipboard

Add API to support editing template files via Admin Interface

Open ashmaroli opened this issue 7 years ago • 11 comments

Hello guys, Yet another PR to augment API -- support editing template files via the admin interface.

(Had to redefine / delegate many existing methods to reach this state. Would love inputs on better alternatives.)

ashmaroli avatar Apr 19 '17 15:04 ashmaroli

@mertkahyaoglu what are your thoughts on providing the ability to edit theme files (layouts, includes, sass, etc.)? I believe this came up before, and at least at the time, we felt that since these were HTML/CSS/Javascript/CoffeeScript/Sass files, and not Markdown, the editing experience wouldn't ever be as good as the experience you'd get in a dedicated code editor, and didn't want the Markdown editing experience to suffer as a result. In my mind, the core use case is to make the 99% day-to-day of running a Jekyll site easier, which is about authoring, editing, and managing content, versus the 1% of the time when you're editing the layouts and site presentation. That said, if you're on board, I'm not strongly opinionated here, and think regardless of how it's handled within the editor, could make sense as part of the API (e.g., a "choose a layout" dropdown).

@ashmaroli Thanks for yet another great pull request. My only suggestion is that wherever possible, we should be using existing Jekyll objects, rather than making our own terminology hashes from scratch in the router. Put another way, what's a template? Why is a SCSS file a template? What would happen if I try to load a JPEG file in the editor? Instead, I'd much rather see dedicated endpoints for the key Jekyll objects we're missing (e.g., layouts, includes) which builds upon the native Jekyll objects. That way, for example, if a user edits a layout, we can reload just that layout and rebuild the site, return additional metadata the Jekyll object already knows, etc. Assets without front matter should be returned via the static files API and assets with front matter via the pages API.

benbalter avatar Apr 19 '17 17:04 benbalter

I'd much rather see dedicated endpoints for the key Jekyll objects we're missing (e.g., layouts, includes) which builds upon the native Jekyll objects.

@benbalter First of all, thank you for your suggestions. AFAIK, Jekyll doesn't have a dedicated object for includes, and assets in the Core. layouts are collected under site.layouts but it will also include layouts from an active theme-gem (which I do not want to be exposed via this endpoint). sass is (most probably) handled by the official inclusion jekyll-sass-converter. But again, the collective array also includes sass files from the active theme-gem.

To support editing just the "template" files at source_dir (editing files in the theme-gem is understandably pointless), I saw just two options:

  • either create a new JekyllAdmin::<Klass> for files under _layouts, _includes, _sass and assets and pass APIable and URLable methods to the entries, or,
  • create custom hashes and pass that to json (implemented in this PR).

The folders under scrutiny here can be grouped under a more logical name Theme but that is already reserved for the similar collective in a gem-based package. So opted for Templates

--

Assets without front matter should be returned via the static files API and assets with front matter via the pages API

While StaticFilesAPI does return assets without front matter, the PagesAPI will only return Page objects that output to .html

--

if a user edits a layout, we can reload just that layout and rebuild the site, return additional metadata the Jekyll object already knows

Unfortunately since jekyll-watch is disabled, I feel this can only be achieved by adding a site.process to the PUT action.

ashmaroli avatar Apr 20 '17 05:04 ashmaroli

Also I'll be adding commits related to the Front End in this PR itself in the coming days, so that you guys can simply review the complete picture with one call to server-frontend

ashmaroli avatar Apr 20 '17 05:04 ashmaroli

In my mind, the core use case is to make the 99% day-to-day of running a Jekyll site easier, which is about authoring, editing, and managing content

@benbalter I agree with you 👍

Jekyll Admin's main purpose is to help authoring, editing, and managing content, not development part of themes. It is always better to prefer IDEs for that purpose (like you said, there might be lots of languages to deal with). So let's focus on daily use case at least for now.

@ashmaroli Absolutely there is a great work here. Thank you for being productive to Jekyll Admin as always. I really liked the key-value part for this PR. I think we can give a general info about the current theme that a Jekyll site uses.

mertkahyaoglu avatar Apr 20 '17 11:04 mertkahyaoglu

I really liked the key-value part for this PR. I think we can give a general info about the current theme that a Jekyll site uses.

@mertkahyaoglu Thank you, but I think you posted this comment in the wrong PR. This PR is about editing template files at source_dir and not about files in a theme :smile:

ashmaroli avatar Apr 20 '17 11:04 ashmaroli

@ashmaroli Yeah, that part belongs to other PR 😄

mertkahyaoglu avatar Apr 20 '17 13:04 mertkahyaoglu

Hi, I've pushed a preliminary commit to initialize the frontend aspect of this PR. Please start up the server-frontend and take a tour when you get some time :wink: I've also considered Ben's suggestions in earlier comments.

ashmaroli avatar Apr 20 '17 15:04 ashmaroli

Jekyll doesn't have a dedicated object for includes, and assets in the Core.

Good point. For includes, I think we can make our own, like we do data files. Are assets not just Pages or static files?

While StaticFilesAPI does return assets without front matter, the PagesAPI will only return Page objects that output to .html

What about an optional argument for the pages API to return non-HTML files?

benbalter avatar Apr 20 '17 20:04 benbalter

Are assets not just Pages or static files?

Yes, they're. But then wouldn't the API url for these files be different?

 _layouts/default.html : http://localhost:4000/_api/templates/_layouts/default.html
_includes/include.html : http://localhost:4000/_api/templates/_includes/include.html
       _sass/test.scss : http://localhost:4000/_api/templates/_sass/test.scss
  assets/css/main.css  : http://localhost:4000/_api/pages/assets/css/main.css
   assets/img/logo.png : http://localhost:4000/_api/static_files/assets/img/logo.png

What about an optional argument for the pages API to return non-HTML files?

That sounds like a nice idea.

ashmaroli avatar Apr 24 '17 14:04 ashmaroli

What I'm suggesting:

 _layouts/default.html : http://localhost:4000/_api/layouts/default.html
_includes/include.html : http://localhost:4000/_api/includes/include.html
       _sass/test.scss : http://localhost:4000/_api/pages/_sass/test.scss
  assets/css/main.css  : http://localhost:4000/_api/pages/assets/css/main.css
   assets/img/logo.png : http://localhost:4000/_api/static_files/assets/img/logo.png

benbalter avatar Apr 24 '17 15:04 benbalter

oh.. so new namespaces for each directory, eh? Nice. Will also add /drafts/ to this list then. Now, how is this API going to be imported into the frontend?

ashmaroli avatar Apr 24 '17 15:04 ashmaroli