Skip to content

Conversation

catilgan-nextension
Copy link
Contributor

Since the issue #587 still open, i tried to implement this missing feature. We needed this feature to translate the linked media files (Audio only) in our project as soon as possible.

Unfortunately, due to the missing deep knowledge in Wagtail module development process and time constraints, I've only implemented the necessary changes for this feature and skipped the unit tests. Furthermore, I couldn't test the video files, since we only need audio files in our project. I believe that the video files should work fine as well.

Copy link
Collaborator

@zerolab zerolab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for starting on this @catilgan-nextension
Had a quick look and left a few notes and suggestions.

This will need a few tests too.

tox.ini Outdated
deps =
wagtail>=5.2,<6.3
Django>=4.2,<5.2
wagtailmedia>=0.0.0,<2.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to move to dev dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the "wagtailmedia" dependency from tox.ini into pyproject.toml under the testing label.

window.chooserUrls = {
imageChooser: "{% url "wagtailimages_chooser:choose" %}",
documentChooser: "{% url "wagtaildocs_chooser:choose" %}",
mediaChooser: "{% url "wagtailmedia:chooser" %}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this work without the hard dependency, we'll need to move the chooserUrls logic to a template tag and conditionally add mediaChooser.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As suggested, I create a new template tag method which checks the wagtailmedia installation and adds the "wagtailmedia:chooser" conditionally into the translation templates. My local tests did work on test environment instance.

elif isinstance(block, ImageChooserBlock):
return {"type": "image_chooser"}

elif isinstance(block, AudioChooserBlock) or isinstance(block, VideoChooserBlock):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrap in an if apps.is_installed("wagtail.embeds") and move the block imports inline

if apps.is_installed("wagtail.embeds"):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the blocks into an inline import if the wagtailmedia has been installed. The code structure didn't change.


elif isinstance(block, AudioChooserBlock) or isinstance(block, VideoChooserBlock):
return {"type": "media_chooser"}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, need to check whether the direct field (that is the ForeignKey) is to a media class and set the widget type for it as well in widget_from_field(), a la

elif issubclass(field.related_model, AbstractDocument):
return {"type": "document_chooser"}

Copy link
Contributor Author

@catilgan-nextension catilgan-nextension May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above - same procedure applied here.

@catilgan-nextension
Copy link
Contributor Author

@zerolab Sorry for my late response. I was very busy and occupied with a different project. Nonetheless, I made the requested changes.
Could you please take a look? Thanks for review and testing.

@kaij
Copy link

kaij commented Jul 11, 2025

@zerolab Any chance to get this merged?

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

Successfully merging this pull request may close these issues.

3 participants