-
-
Notifications
You must be signed in to change notification settings - Fork 186
Description
Description
I am not 100% sure if this is a bug report or feature request. For our use-case it is a blocking issue in using uuid based links and an inconsistency in link handling that i'd love to discuss and fix. We are currently migrating our full site from hardcoded urls to page:// based links. This works fine most of the time and we are super happy to make page renames easier in the future. One thing that is blocking us from generally using this method is, that we are loosing all anchors from all of our links, which breaks a lot of content on our site.
Before we used:
(link: /content/#myanchor text: Test)
Now we would like to migrate to:
(link: page://EWQEPJBB2Xyrrrj6#myanchor text: Test)
But the output on the rendered site will just eat up the anchor. The link tag also does not allow an isolated definition of anchors. Therefore i think this is inconsistent and feels more like a subtle bug.
Expected behavior
Even if using uuid based links we should be allowed to use anchors (and maybe even query parameters if we want some javascript magic to happen)
Proposed patch (Our quick fix for now)
diff --git a/kirby/config/tags.php b/kirby/config/tags.php
index 9f98127..9619c53 100644
--- a/kirby/config/tags.php
+++ b/kirby/config/tags.php
@@ -210,7 +210,11 @@ return [
Uuid::is($tag->value, 'page') === true ||
Uuid::is($tag->value, 'file') === true
) {
- $tag->value = Uuid::for($tag->value)->model()?->url();
+ $anchor = null;
+ if (preg_match('/(#[^#]*)$/', $tag->value, $matches)) {
+ $anchor = $matches[1];
+ }
+ $tag->value = Uuid::for($tag->value)->model()?->url() . $anchor;
}
If this patch is good enough i can submit a PR. Feel free to just use the proposed solution. No credit required.
To reproduce
Just add a kirby tag with an anchor
(link: page://EWQEPJBB2Xyrrrj6#myanchor text: Test)
Render the page and the anchor will be lost.
Your setup
Kirby Version
Current version from starterkit (8419d9e37360c12029375ef44cb9499323d4098a)