camya / filament-title-with-slug

"Title With Slug" Input - Easy Permalink Slugs for Filament Forms (PHP / Laravel / Livewire)
MIT License
130 stars 39 forks source link

Filament v3 Support (Fixes #23) #24

Open Log1x opened 1 year ago

Log1x commented 1 year ago

Change log

Screenshot Screenshot 2

Log1x commented 1 year ago
Camya\Filament\Forms\Components\TitleWithSlugInput::Camya\Filament\Forms\Components\{closure}(): Argument #2 ($set) must be of type Closure, Filament\Forms\Set given

Working on figuring out some changes with $set before this can be merged.

Log1x commented 1 year ago

Hmm this is very close to working. The exception above is fixed but when editing the slug it's not showing up once you press "OK" despite it properly storing it in the hidden field.

The slug is properly getting generated when creating an initial title, too.

howdu commented 1 year ago

@Log1x I've pushed a couple of commits you can review.

Adding $wire.set to manually push the changes fixed the issue you were having.

Log1x commented 1 year ago

nice!! i appreciate it

camya commented 1 year ago

Awesome, nice work @Log1x and @howdu. Thank you for the pull request. I just tested it a little bit.

I found this problem: If I type fast, the form skips some characters. It looks like the form is saved on each key stroke. (see flickering create button). I'm not shure what causes this. (See video below)

Video:

https://github.com/camya/filament-title-with-slug/assets/827658/46c72fc6-5f16-4a1f-94b2-4f03cd89ec30

Log1x commented 1 year ago

I don't think it's actually saving, I think it's just swapping a network request to update the state due to ->reactive() on the inputs.

It'd probably be a safe bet to change this to ->live(true).

Edit: I'm not seemingly able to reproduce the missed characters. Edit 2: Nevermind, yes I am during Creation.

Log1x commented 1 year ago

Updated the test stuff a bit but 3 of them are still failing due to https://github.com/livewire/livewire/blob/main/src/Features/SupportModels/ModelSynth.php#L20-L22

As far as I can tell the only fix for this on Livewire 3 is to do a database migration for the tests so we can create the Record using a RecordFactory. ->make() and every other approach I could possibly think of does not work.

Log1x commented 1 year ago

Ok I reproduced the character skipping when on a resource Create page. ->live(true) instead of ->reactive() is a "fix" but I think there's something else going on because it should be fine either way.

Log1x commented 1 year ago

Hmm, I'm not sure. I never used the plugin in v2 so I don't know how it behaved, but commenting out the $set for the field slug fixed the issue as well as doing ->live(true) instead to only update the slug on blur.

It's possible this is a weird quirk with Livewire 3 since it tries to bundle network requests. If we have 1 form getting live updated while also $setting state on another component, I think it's overriding its self or something. I could be completely wrong though.

howdu commented 1 year ago

I can't reproduce the flickering but I think the hidden input can be removed now it's manually being set from $wire.set

gafriputra commented 1 year ago

can't wait for this PR to merge 🚀

faab007nl commented 1 year ago

can't wait for this PR to merge 🚀

Same here

Log1x commented 1 year ago

I changed the input reactivity to use blur due to Livewire's network requests not being able to keep up with typing when on a "Create" page where it will auto-generate the slug for you. I think having it as blur still feels good and I'm having no issues on my end.

Again, this issue only came into play on a Create page where the slug would get auto-generated based on the title you enter. Having it spam network requests to Str::slug() the slug in real-time while typing was otherwise causing it to not be able to keep up.

I also removed the hidden input that @howdu mentioned and everything seems good in my app.

I think it's possible for the slug_auto_update_disabled stuff to possibly get replaced as well but it's not a necessary change and everything works good as-is.

@camya do you want me to do a test database migration and fix the final 3 tests?

stvtk commented 1 year ago

@camya @Log1x Do we know this PR is getting merged, please?

simplyaakif commented 1 year ago

Any update on this. ? We appreciate you guys working on this. We have a few resource that needs the v3 Update.

chosten commented 1 year ago

A few suggestions:

Log1x commented 1 year ago

If you want to do a quick PR against https://github.com/Log1x/filament-title-with-slug/tree/feat/filament-v3 I will merge it in – otherwise I don't mind doing these changes.

I also don't mind forking and maintaining this if @camya doesn't want to anymore. For now, I'm using it in my projects by adding the repository to my composer.json:

{
  "require": {
    "camya/filament-title-with-slug": "dev-feat/filament-v3"
  },
  "repositories": [
    {
      "type": "vcs",
      "url": "https://github.com/log1x/filament-title-with-slug"
    }
  ]
}
maikelmotivo commented 1 year ago

@Log1x I think it is pretty self evident that the op has lost interest in maintaining this package by now. I'd be very happy to see you fork and maintain this project!