search

medienbaecker / kirby-modules

Plugin for modular Kirby websites
MIT License
73 stars 7 forks source link

Enable inline previews + Kirby 3.7 compatibility #33

Closed nilshoerrmann closed 2 years ago

nilshoerrmann commented 2 years ago

This pull request adds a custom layout type module for modules sections which can be used by other plugins to customize the item layout, e. g. enabling inline editing: https://github.com/hananils/kirby-modules-inline (still experimental, but hey, it seems to work 🎉).

Furthermore, it updates the codebase for Kirby 3.7 and removes the modules page from the breadcrumb.

height[bot] commented 2 years ago

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

nilshoerrmann commented 2 years ago

Oh, I just saw my commits mess with the whitespace. I'll have to check what's wrong there …

medienbaecker commented 2 years ago

Thanks for the PR, Nils 👍

Great stuff, I already fixed the space/tab discrepancies and created a pre-release for your changes.

Two things I noticed:

  1. Overwriting the parents method to remove the modules container from the breadcrumb is very clever! It will be a breaking change though. I have some projects that rely on the parent() or parents() method on modules — and I guess I'm not the only one. But I think it's worth it. The change in the core I'm waiting for doesn't seem to be high priority: https://github.com/medienbaecker/kirby-modules/issues/31
  2. I saw you also overwrote the parents method for the modules container. Was this a mistake or am I missing something?
nilshoerrmann commented 2 years ago

I saw you also overwrote the parents method for the modules container. Was this a mistake or am I missing something?

Looks like a copy and paste error ☺️

nilshoerrmann commented 2 years ago

I have some projects that rely on the parent() or parents() method on modules — and I guess I'm not the only one.

That interesting – may I ask in which context you need parents() with modules and how the modules parent is required in this context. My thought was that this container page wouldn't be needed in the parents context.

medienbaecker commented 2 years ago

I just had a look at a few projects and I think you're right. It seems like I only ever use parents()->last() which isn't affected by the filtered collection.