fabianmichael / kirby-markdown-field

Super-sophisticated markdown editor for Kirby 3, community built.
Other
160 stars 14 forks source link

Add ability to set per-extension query and info for pages picker #173

Closed s3ththompson closed 1 year ago

s3ththompson commented 1 year ago

This PR adds the ability to configure additional options for the builtin pagelink button, via a new top-level pages property (mirroring the top-level files and uploads options). This new property supports more options than just query, including info and text.

text:
  type: markdown
  pages:
    query: site.index
    info: "{{ page.tags }}"
  buttons:
    - pagelink

Additionally, this PR adds the ability for extensions to support their own pages, files, and uploads properties. For example, you could add multiple extensions to add multiple buttons to link different types of pages.

text:
  type: markdown
  files:
    […]
  pages:
    […]
  buttons:
    pagelink:
      pages:
        query: site.index
        info: "{{ page.tags }}"
    albums:
      pages:
        query: site.find('albums')
        info: "{{ page.tags }}"

To implement this feature I added an optional route pattern for the markdown field's pages, files, and uploads routes: /(:any)/pages, /(:any)/files, and /(:any)/uploads where (:any) is the name of the extension and the corresponding key in the blueprint buttons property.

Extensions can open a pages picker, for example, that reads the appropriate query and info properties by setting the endpoint appropriately:

this.editor.emit("dialog", this, {
  endpoint: this.input.endpoints.field + "/albums/pages",
  multiple: false,
  selected: [],
});

I also updated the README and added an example of a custom pagelink extension.

Note: This PR also soft-deprecates the existing query option (by removing it from the documentation, but still reading the value as a fallback)

fabianmichael commented 1 year ago

@s3ththompson Thanks for this PR. I am just a bit confused about the options being placef at top-level while using such generic keys. I would rather recommend mocing these to a subkey or to the button configuration itself:

text:
  type: markdown
    pagepicker:
      query:
        pagelink: site.index
        albums: site.find('albums') # used by custom extension, albums
      info:
        pagelink: "{{ page.tags }}"
        albums: "{{ page.title.uppercase }}" # used by custom extension, albums

or

text:
  type: markdown
  buttons:
   pagelink:
      query: site.index
      info: "{{ page.tags }}"

What do you think?

s3ththompson commented 1 year ago

Thanks for the quick reply. Your Option 2 (query and info are options that are specified under the button configuration) makes far and away the most sense to me.

text:
  type: markdown
  buttons:
    pagelink:
      query: site.index
      info: "{{ page.tags }}"

I was trying to avoid changing the location of the existing top-level query option for pagelink. I'm assuming if we went with Option 2 you would want to continue supporting the existing location for backwards compatibility, but update the documentation to encourage setting these options under the button configuration instead (for both pagelink and custom extensions)?

Happy to update the PR if you're okay with this.

fabianmichael commented 1 year ago

What about using the classic location for everything to keep configuration compatible with the regular textarea field and add the custom route for other plugins and let those define their configuration with the corresponding button?

s3ththompson commented 1 year ago

I think I follow. Just to be sure can you share a snippet of what that would look like in YAML?

When you say “classic location for everything to keep configuration compatible with the regular textarea field” do you mean for the query (and info) properties? (AFAIK textarea doesn’t have those properties, so I just want to make sure we’re talking about the same thing.)

fabianmichael commented 1 year ago

@s3ththompson I just looked it up and the default textarea uses the files property (see Kirby reference), so we should probably follow that to establish a seamless transition from the default field, same goes for pages. Maybe you could have a look at the Kirby 4 alpha? As far a I know, it has advanced page picker support and maybe it already contains additional options?

This also makes sense, because other editor plugins (e.g. inline image preview, autocomplete, …) could always use that option value, no matter if the file button is present or not. All other plugin configuration (e.g. custom page picker button) should be part of the button configuration … does that make sense?

text:
  type: markdown
  files:
    […]
  pages:
    […]
  buttons:
   pagelink:
      query: site.index
      info: "{{ page.tags }}"
s3ththompson commented 1 year ago

I completely agree with your suggestion. I think it makes things much more clear to follow the textarea convention established with the files option. (The new Kirby 4 Link field has an advanced pages picker but defaults to the entire site index. There is no support yet for querying custom pages, so no conflicts there.)

I made only a small change to your YAML example above. I added a pages property under the pagelink button configuration, so that we can support extending pages, files, and uploads per extension.

text:
  type: markdown
  files:
    […]
  pages:
    […]
  buttons:
    pagelink:
      pages:
        query: site.index
        info: "{{ page.tags }}"

I also updated the README to describe the new pages option (and the new support for extensions to override the base configuration for pages, files, and uploads).

The only wrinkle, is that rationalizing the config options means deprecating the current query option (in favor of pages). I made sure the code is backwards compatible, but you may still want to add a note to the README or make this change a minor version release.

Let me know if you have any other feedback.

fabianmichael commented 1 year ago

@s3ththompson This looks fine, I’ll just merge it right-away! Thanks for your contribution.