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

Basic implementation that changes the Blocks formwidget to extend MLR… #34

Conversation

nmiyazaki-chapleau
Copy link

Basic implementation that changes the Blocks formwidget to extend MLRepeater and customizes the getLocaleSaveData to add the _group and _config to the save data. Also changes all blocks to use the Translate version (ml*).

This should be used alongside my PR for the wintercms/wn-translate-plugin's wip-ml-repeater-fields draft PR branch, at least until it gets merged into the draft PR branch.

This implementation is basic and very very rough around the edges, but is a working proof of concept.
This POC forces users to use Translate. Ideally, there should be an easier way to override the getLocaleSaveData and extend the MLRepeater without forcing its use. A potential idea could be to just have an MLBlocks formwidget alongside the Blocks formwidget and add blocks that implement the ML versions.

I'm not a fan of using the formWidgets' data key as it is deprecated and should only be for backwards compatibility, but I did not find a simpler way for now.

The best implementation IMO would be to be able to add a translatable property to block fields which would make them use the ML version of their formwidget. I am not sure how the translatable property implementation in the wip-ml-repeater-fields branch is going, however.

I will gladly take any ideas or suggestions to improve this!

…epeater and customizes the getLocaleSaveData to add the _group and _config to the save data. Also changes all blocks to use the Translate version (ml*)
@mjauvin
Copy link
Member

mjauvin commented Nov 4, 2024

@nmiyazaki-chapleau you cannot force this in here... not everyone wants to use the Translate plugin. This needs to be done in the Translate plugin itself with events.

@nmiyazaki-chapleau
Copy link
Author

@mjauvin Very much aware! I just don't have the time to invest into a fully-fledged version. As mentionned, this is more of a proof-of-concept.

@mjauvin
Copy link
Member

mjauvin commented Nov 4, 2024

Then why submit this at all (and in here) ?

@LukeTowers
Copy link
Member

@nmiyazaki-chapleau I appreciate you sharing what you've come up with so far, but @mjauvin is correct, anything related to the translate plugin should be done in the translate plugin itself. I'll close this PR but it'll still remain here for others to reference if they want to finish the work you started.

@LukeTowers LukeTowers closed this Nov 5, 2024
@nmiyazaki-chapleau
Copy link
Author

You guys are correct, sorry about that!

@LukeTowers
Copy link
Member

No worries :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants