robinscholz / better-rest

Kirby plugin for better REST requests
MIT License
51 stars 3 forks source link

Improvement: Ship alt or caption with srcset (or whatever fields are associated with image) #8

Closed Jones-S closed 4 years ago

Jones-S commented 4 years ago

Hi. And it's me again... 😅

I just setup an image blueprint to allow adding alt tags and captions. In a REST setup you really would like to have this kind of information directly delivered with the image. (I would not want to make a second request to get the files of a specific page, where this information is included).

So it would be great if all fields would come with the image, along with the srcset.

If you're in lockdown (corona) and you feel like fixing this, that would be great. Maybe I will try to make a PR, but I am not very good with kirby nor PHP... :D

Anyway, let me know what you think.

Cheers

Jones

Jones-S commented 4 years ago

Hm I messed around a bit and thought I could for testing purposes extend your applySrcSet function.

I found this post in the forum and thought something like this should work:

/**
     * @param $value
     * @return array
     */
    public function applySrcSet($value): array
    {
        $srcset = \Kirby\Toolkit\A::get($this->options, 'srcset');
        if (! $srcset) {
            // $value['srcset'] = null; // NOTE: not setting is better for frontend than `null`
            return $value;
        }

        $file = $this->kirby->file($value['id']);
        $value['srcset'] = $file->srcset($srcset);

        $c = $file->content();

        foreach ($c as $key => $value) {
          if ($key == "fields") {
            foreach ($value as $field) {
              $value[$field] = $file->$field();
            }
          }
        }

        return $value;
    }

Unfortunately nothing happens. Also I can' t really test things by printing a few values to the screen, because as soon as I don't return json I get an error "message": "Return value of Robinscholz\\Betterrest::applySrcSet() must be of the type array, string returned",

what I found out is:

        $c = $file->content();

        var_dump($c->data());

If I use this I see that the fields are within this data() which I've never seen in kirby before... I'll try to keep digging though....


Funnily this works:

// within your function applySrcSet where I have access to the file:

 $c = $file->content();
$value['alt'] = $c->alt();

This will give me my alt value like:

"alt": {
     "value": "My alt text"
}

Not exactly how I want it. But still I don't understand why the for-each loop above would not work...

I realized that something like

$c = $file->content();

        foreach ($c as $key => $value) {
          if ($key == "data") { // here I should check for data instead of fields...
            foreach ($value as $field) {
              $value[$field] = $c->$field(); // but this does not help at all
            }
          }
        }
robinscholz commented 4 years ago

I see a few problems with this. Since you don’t know what someone will put into their files blueprint, you can’t really predict if the content needs to be simply returned or transformed as well with i.e. kirbytags or smartypants.

Since this would slow response times, it would need to be included as a seperate option, turned off by default. Hooking into the srcset function is also a bit obscure.

Since you can already fetch this info with a seperate api call to i.e. /rest/pages/:id/files, I don’t see the need to include this for now. However, feel free to submit a PR!