fabianmichael / kirby-markdown-field

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

[next| Compatibility with Kirby 3.6 #116

Closed fabianmichael closed 2 years ago

fabianmichael commented 3 years ago

It needs to be tested, whether the field is compatible with the upcoming Kirby version. At least the font option does not work at the moment.

herrajon commented 3 years ago

Neither does the link option

undefined is not an object (evaluating 'this.$store.state.system.info')

medienbaecker commented 3 years ago

I looked at the breaking changes and it looks like this has to be changed for the button to work.

The Vuex store and store modules have been changed: https://getkirby.com/releases/3.6/breaking-changes#panel__helpers-libraries

afbora commented 3 years ago

I don't know if it has anything to do with 3.6, I didn't look it in detail. Getting following error when hr button clicked:

this.syntax.render is not a function

medienbaecker commented 3 years ago

this.syntax.render is not a function

It looks like this is a different issue. I could reproduce it in 3.5.4.

this.syntax.render() is indeed not a function and using render instead of render() works perfectly fine: https://github.com/sylvainjule/kirby-markdown-field/blob/next/src/components/Buttons/HorizontalRule.js#L23

I added a fix to the PR anyway.

bastianallgeier commented 3 years ago

The font issue is weird because it's actually a problem in the PHP implementation. The font prop only accepts a string on the next branch. As soon as you pass an array like in the docs, the setting is causing a type exception

medienbaecker commented 3 years ago

The font option actually works perfectly fine for me. Keep in mind the README is different for the next branch: https://github.com/sylvainjule/kirby-markdown-field/tree/next

You can either pass monospace or sans as a string.

bastianallgeier commented 3 years ago

@medienbaecker I completely missed that. @fabianmichael @sylvainjule is there anything I can help you with? It looks like it's working already pretty well, except the hr button.

fabianmichael commented 3 years ago

@medienbaecker @bastianallgeier @herrajon @afbora @sylvainjule I took a few minutes and merged all PRs this noon and also added quite a few fixes. My plan would be to focus all development work on 3.6+. The plugin uses some of Kirby’s custom properties and I’m not willing to maintain this for 3.6 and legacy versions. Limiting support to Kirby 3.6 would also allow us to implement the really cool things like Autocomplete for Kirbytags (especially image and file tags) and inline previews for (image: ...)` and maybe video/oembed and gallery tags in the future.

With CodeMirror as our backend, sky is the limit. We only need to decide at some point, whether this field should also provide any backend stuff like additional Kirbytags or if all of any extra features that would extends the scope of a single field should become separate plugins.

My time is currently rather limited, so I’m very happy for anyone who can provide feedback, testing and PRs are highly welcome. If anyone knows how to set Kirby 3.6 as minimum version, please create a PR.

I don’t know about Sylvain’s interest in continuing his work on the plugin, so it might also be an idea to move or copy the repo to my GitHub account, as basically all of the next version has been developed and maintained by me so far.

lukasbestle commented 3 years ago

@fabianmichael The Versions plugin checks the Kirby version like this, maybe it can be an inspiration:

https://github.com/lukasbestle/kirby-versions/blob/f25c50d0e73afadba9dda0dab5ef936aeec20480/composer.json#L14

https://github.com/lukasbestle/kirby-versions/blob/f25c50d0e73afadba9dda0dab5ef936aeec20480/index.php#L18-L32

I agree it makes sense to make a clear cut sometimes. After all the previous plugin versions will still work with older Kirby versions if it's really necessary.

fabianmichael commented 3 years ago

@lukasbestle Thanks for your input, I just added the version check to the latest version of the plugin. I also updated some variable names like font families to their new 3.6 names.

I’m sorry to cut support for older Kirby versions at this point, but using the new custom properties without any fallbacks will ease future development a lot. Maybe it will also be possible to rely more on Kirby’s color system without all the custom additions I had to add for syntax highlighting etc. But this can wait a bit, until the new colors have added to the reference.

cc @sebastiangreger @jonathanmuth

fabianmichael commented 2 years ago

Does anyone still have issues with 3.6.x? If not, I would like to close this ticket. The final release of 2.0 of this field should happen quite soon now … :-)

sebastiangreger commented 2 years ago

@fabianmichael I have not worked with Markdown texts a lot since updating to 3.6, but at least haven't noticed any obvious issues. There are a few little glitches that I haven't really been able to pinpoint quite yet, but those are merely aesthetic and likely not 3.6-related (I'll try to follow up on them in separate issues at some point).

fabianmichael commented 2 years ago

@afbora @bastianallgeier @lukasbestle As there have been no further complaints and I’ve been using the plugin with 3.6 for some time now, I’ll close this. It’s safe to assume, that the Markdown Field 2.0 is fully compatible with Kirby 3.6.