filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
17.73k stars 2.78k forks source link

Media library doesn't seem to work with (repeaters and) builders #1284

Closed ralphjsmit closed 2 years ago

ralphjsmit commented 2 years ago

Package

filament/spatie-laravel-media-library-plugin

Package Version

v2.9.6

Laravel Version

v8.79.0

Livewire Version

v2.9.0

PHP Version

PHP 8.1

Bug description

I'm using the Builder component with a SpatieMediaLibraryFileUpload inside it. The stripped down version looks like this. I noticed that when the same block appears multiple times in a Builder, the same image will be attached to it.

Schermafbeelding 2022-01-18 om 18 23 37

It would be great if this tool would support a way of adding multiple image blocks, each with it's own separate image, for example with custom properties.

Steps to reproduce

Builder::make('body')
->blocks([
    Builder\Block::make('image')
        ->icon('heroicon-o-photograph')
        ->schema([
            SpatieMediaLibraryFileUpload::make('image')
                ->label('Label)
                ->collection('content')
                ->required(),
        ]),
]),

Relevant log output

No response

danharrin commented 2 years ago

I think you need to look at this a different way. The reason why media library exists is to associate files with Eloquent models. You're looking to associate models with a certain key inside a column inside the Eloquent model. Spatie's package doesn't support that behaviour - it's like requesting us to add relationship support to this JSON column :)

Hopefully you can see how this is not really an issue - instead a bit of a misunderstanding about how media library is meant to be used. What are the disadvantages of using a FileUpload here with the same disk as media library?

ralphjsmit commented 2 years ago

Hey Dan, makes sense. Thanks for explaining!

The disadvantages of a FileUpload are that a FileUpload doesn't automatically handle file size conversions for you:

    public function registerMediaConversions(Media $media = null): void
    {
        foreach ([400, 800, 1200, 1600] as $size) {
            $this->addMediaConversion((string) $size)
                ->width($size)
                ->optimize();
        }
    }

I'll solve it in a different way now. Think that I'm gonna build a WordPress-like Media Library plugin with a custom component to select an image.

GarethSomers commented 2 years ago

Hello!

I actually tackled this exact problem but in a different context (Laravel Nova + Nova Flexible Content).

It might be helpful for anyone that wants to solve it here; a) I associated the media with the actual model; e.g. model_type = App\Models\Page and model_id = 1. b) Each repeater row has a unique string identifier (thought it was a UUID but I'm mistaken). c) I set the collection_name value to be that of the collection + the unique string (e.g. image_hDjcvkGTMEa8TggA) d) I added a callback on the repeater that would delete the media from that collection when the row was removed.


I'm not too familiar with this package, I'm just looking at it as an alternative to Laravel Nova but would simplify our transition atleast.

I can see SpatieMediaLibraryFileUpload could be duplicated and/or modified to tackle some of these problems, assuming each row already has a unique identifier, and the method saveUploadedFileUsing can identify what row it's in.

The other problem is removing the media when the row is deleted... It looks like we could hook into the event repeater::deleteItem

@danharrin any reason this wouldn't work, even as a third party package?

danharrin commented 2 years ago

@GarethSomers every Filament method accepts a Closure for dynamic customization. So in theory, it's as easy as

->collection(fn (FileUpload $component) => (string) str($component->getContainer()->getStatePath())->afterLast('.'))
->afterStateHydrated(null)
->mutateDehydratedStateUsing(null)

to save / load an upload into a collection based on the repeater item:

https://filamentphp.com/docs/2.x/forms/advanced#using-closure-customisation

GarethSomers commented 2 years ago

Hey @danharrin I've just got around to looking at this today.

I got it working with something like this:

Builder::make('foo')
    ->blocks([
       Builder\Block::make('bar')
            ->label('Layout - Test')
            ->schema([
                SpatieMediaLibraryFileUpload::make('Test')
                    ->collection(function (FileUpload $component) {
                        return array_slice(explode('.', $component->getContainer()->getStatePath()), -2, 1)[0] . '.media';
                    }),
            ]);
    ])
    ->afterStateHydrated(null)
    ->mutateDehydratedStateUsing(null);

So afterStateHydrated and mutateDehydratedStateUsing is applied to the Builder (or Repeater). And I'm simplified the collection name to be the last uuid (-2 in array slice will get that for a Builder, -1 for repeater).

To resolve this in my Resource's I'm extended a class:

<?php

namespace App\Http\Resources\Modules;

use Illuminate\Http\Resources\Json\JsonResource;

class AbstractModule extends JsonResource {
    public $model = null;
    public $id = null;

    public function setModel($model) {
        $this->model = $model;
        return $this;
    }

    public function setId($id) {
        $this->id = $id;
        return $this;
    }
}

Each parent (i.e. a ModuleBuilderResource) will do setModel and setId for each child (i.e. "TwoColumnsResource")... from their each resource can resolve what media it has like so: $this->model->getFirstMedia($this->id . '.media');


@danharrin Ideally need to delete the media once the repeater item / block has been removed.

I can see there are events like repeater::deleteItem which might help.

In the scenario where we have 2 nested repeaters / builders, would deleting the higher repeater propergate a delete event to the lower repeater?

AlexisSerneels commented 2 years ago

While using ->afterStateHydrated(null)->mutateDehydratedStateUsing(null); works great, an error is thrown while reordering elements.

james2doyle commented 5 months ago

I ended up doing this:

Builder\Block::make($name)
    ->schema([
        SpatieMediaLibraryFileUpload::make('image')
            // uses the hidden image field path OR the current path
            ->collection(function (FileUpload $component, Get $get) {
                return $get('image_collection_id') ?? $component->getContainer()->getStatePath();
            })
            ->afterStateHydrated(null)
            ->mutateDehydratedStateUsing(null)
            ->responsiveImages()
            // sets the hidden image field to the state path OR the previous path
            ->afterStateUpdated(function (FileUpload $component, Set $set) {
                $set('image_collection_id', $component->getContainer()->getStatePath());
            })
            ->live(),
        // we can now call $yourModel->getMedia($value_in_image_collection_id)->first()
        Hidden::make('image_collection_id'),
    ])

Which basically just tracks the state path, uses that as the collection name, and then lets me find the image using that collection, which is the state path.