OblikStudio / kirby-link-field

Links field for Kirby.
MIT License
77 stars 10 forks source link

Legacy string values disappear in templates / are not shown in the planel #60

Closed pstaender closed 2 years ago

pstaender commented 2 years ago

Hi 👋🏻

we are using your link plugin with many legacy values (from previous versions) just stored as plain textfield. These urls do not start necessarily with http because we use absolute url without a base URL, e.g.: /about-us instead of http://mysite.com/about-us. The validator was always fine with that, so that's why it was used that way (and we have different base urls for staging and production).

With version 4.0.1 all values are gone, I guess because of the additional check for string starting with http (https://github.com/OblikStudio/kirby-link-field/commit/5da094a39a2b06de6e7c95c230ee660e43f9f76d#diff-7413d6453f901e939bbd840c8f0d1c7b20c2ca0e7f71741e4e07c6cf036f16c0R213) : all string value which do not start with http are empty after that.

I would suggest, a link field should also allow strings starting not with http for urls, or what do you think?

Here are some quick changes I made, to make it work (just for illustration):

'fields' => [
    'link' => [
        'mixins' => ['pagepicker', 'filepicker'],
        'props' => [
            'value' => function ($input = null) {
                if (is_string($input)) {
                    // Value comes from a TXT file.
                    $data = Yaml::decode($input);
                } else {
                    // Value comes from the panel and is serialized.
                    $data = $input;
                }

                if (empty($data['type']) && is_string($input)) {
                    // Handles cases where the field was previously of type
                    // `url` and still a URL as plain value.
                    return [
                        'type' => 'url',
                        'value' => $data
                    ];
                }
'fieldMethods' => [
       if ($field->isEmpty()) {
    return null;
       }
      $value = $field->value();
      $data = $field->yaml();
    if (is_string($value) && !isset($data['type'])) {
        return new Link([
            'type' => 'url',
            'value' => $value
        ]);
    }

Before:

image

After (which is still showing a warning red, but is ok and stores the value):

image
hdodov commented 2 years ago

Well, in that case, you could just set the link type to "Page" and select your About Us page. The /about-us value is invalid (red), because it's not a valid URL. It has no protocol part and no hostname… In order to make /about-us valid, there should be another link type, such as "Plain href" or something, but that kind of defeats the purpose of the field, which is to abstract the href from the user.

Couldn't you run a script over your content that updates all outdated link values with valid ones? That's what I'd do in that case.

pstaender commented 2 years ago

In our case we have urls which are not pages but exists in the routing of the kirby app: e.g. /shop/prodcuts/:name routes to a product which is fetched via an an api and is not persistent as a page in kirby. Thats why a string as url with /shop/products/abc fulfilled our needs completely and worked fine. Thats why an updating of the value would not work, the values would disappear.

hdodov commented 2 years ago

Then I think that this is not a use case for the link field. You are not actually pointing to a link in your content, you're pointing to a route in Kirby. That's not something that the link field is meant to solve. Wouldn't it make more sense to just have a simple text field?

pstaender commented 2 years ago

I think we will convert all values manually, then everything should be fine. Thanks for your input! And thanks for making this plugin :)