Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: Fix incompatibility issue with hydration hooks in forms (spatie-laravel-translatable-plugin) #11450

Conversation

Voltra
Copy link
Contributor

@Voltra Voltra commented Feb 16, 2024

Description

This PR fixes an issue in the locale switcher that made the form filling behave differently on locale switch than on page load.

Without this PR, packages like awcodes/filament-curator that rely on hydration hooks could not be used in conjunction with filament/spatie-laravel-translatable-plugin's language switcher.

The change is absolutely seemless, and comes from my fiddling on a personal project's vendor files trying to fix said bug.

  • Visual changes (if any) are shown using screenshots/recordings of before and after.

Code style

  • composer cs command has been run.

Testing

  • Changes have been tested.

Documentation

  • Documentation is up-to-date.

@zepfietje zepfietje added the bug Something isn't working label Feb 17, 2024
@zepfietje zepfietje added this to the v3 milestone Feb 17, 2024
@danharrin danharrin self-assigned this Feb 23, 2024
@demianottema
Copy link

Could this also be the solution for #11826?

@Voltra
Copy link
Contributor Author

Voltra commented Mar 14, 2024

Seems likely

@bojmaliev
Copy link

Any hope this will get merged soon?

@danharrin
Copy link
Member

In the meantime, these are methods you can override in your app to see if they fix your issue. This is a very dangerous PR for me to merge and I need to dedicate a lot of time to testing it before it is released.

@bojmaliev
Copy link

@Voltra does this still works for you? I tried to override these files in the latest version of filament and I'm getting an exception.

@Voltra
Copy link
Contributor Author

Voltra commented Mar 21, 2024

@bojmaliev I just updated to the latest Filament version and re-applied the patch, it still works as expected

@atmonshi
Copy link
Contributor

hi @Voltra, I haven't test this PR, but seeing the changes are similar to #12372

we cant avoid filling the data see my comment

can you test your code with an upload file?!

@Voltra
Copy link
Contributor Author

Voltra commented May 2, 2024

Haven't tested, but know that the way the form data is filled in right now will also mutate data (since it's the form's "data path"), we just go about it using the form's usual behavior instead of working with the raw data directly

@atmonshi
Copy link
Contributor

atmonshi commented May 9, 2024

hey :)

did more testing on your changes
still the same the file upload will be lost after switch the language.

so the issues for the upload is because $this->form->getState() will return the final state which is the file name, when we need the file upload obj to set it to the file component after switching the lang.

can you test your use case with awcodes/filament-curator and use:
$this->form->getRawState()

it seems to work.

edit: adding some debugging screenshot

Screenshot 2024-05-09 at 10 18 33 PM

@danharrin
Copy link
Member

I am posting this same message across all Spatie Translatable issues & PRs

Hey all! Wanted to update you on where we're at with the translatable plugin.

I really appreciate your patience while this issue has been active. While I created the plugin for the community, I've never actually had a project where I needed to use it, and that's the same for the rest of the Filament team. As such, it's the reason why the plugin hasn't had as much attention as the others, and is much more unstable: I just don't have the environment to test all the use cases, nor the motivation to make it truly great.

As such, I put out a post a month ago and asked who in the community uses the plugin and has knowledge of plugin development. Luckily, Lara Zeus and Mohamed Sabil stepped forward, who are both authors of popular Filament plugins and are trusted by the community.

We are strongly considering handing over maintenance of the plugin to those developers, for the good of the community. You can find their fork at https://github.com/lara-zeus/translatable.

Since the new developers have lots of experience with the plugin and active projects that use it, they should be able to help debug issues easier and make a much more stable experience for other users.

If their work goes well until v4 is released, we will likely retire the plugin at that point and recommend the fork as an official replacement. If it does not go as expected and the community is unhappy, then we will reconsider this decision. I am closing this for now, and if we decide to take maintenance back officially then we will probably reopen it.

The existing plugin will continue to receive security updates indefinitely. Please let me know if you have any further questions.

Many thanks,
Dan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high risk PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants