Problem/Motivation

- I have a node content type with a paragraph which has a reference with Media (all are translatable).
- I have created a media and translated this media's alt text.
- I've assigned that media to the paragraph (on all languages).
- When the paragraph is rendered with the photoswipe format I get the alt text in the default language. Even when viewing the node on another language.

Proposed resolution

I've attached a patch in an attempt to fix this.

Remaining tasks

I'm unsure if this is the best way to fix this. Maybe someone can provide feedback.

CommentFileSizeAuthor
media_translation.patch947 bytesdave.mentens

Issue fork photoswipe-3576726

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dave.mentens created an issue. See original summary.

dave.mentens’s picture

dave.mentens commented
Status: Active » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

@dave.mentens please use a MR not patches.

LGTM in general but I think there was a better method for this, I have something with context in mind...

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Yeah I think maybe

/** @var \Drupal\Core\Entity\EntityInterface $entity */
$entity = $entity->getTranslationFromContext();

is more appropriate?

dave.mentens opened merge request !142

dave.mentens’s picture

dave.mentens commented

@anybody Thanks for the feedback. I've updated my change and made a MR.

dave.mentens’s picture

dave.mentens commented
Status: Needs work » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

Tests are failing and shouldn't this use dependency injection maybe?

stred made their first commit to this issue’s fork.

stred’s picture

stred commented
Status: Needs work » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil

I'm biased. Maybe was wrong, because a DTO should be "dumb" and maybe not use DI - but on the other hand it makes sense for texting, because the DTO still needs the dependency within.

The key fix of this issue is in this line:
https://git.drupalcode.org/project/photoswipe/-/merge_requests/142/diffs...

@grevil what do you think? I tend to merge it as-is, because I think there's no real right or wrong here.

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs review » Needs work

Not a big fan of the dependency injection change. Even if we have the helper method "createFromVariables", the constructor is public. Meaning all "new ImageDTO" calls now require a second parameter, which would require a deprecation notice for now "calling ImageDTO, without "entityRepository" is deprecated in x and will be removed in y.

And (if we'd be 100% version conform) we'd need a new major version, where we remove the parameter entirely. And all this for introducing an inconvenience.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @grevil okay then let's just keep the key change!
Sorry @stred - can you prepare a separate MR with just the required change maybe? So we have both and can compare the changes.

grevil’s picture

grevil commented

No need for that, we can simply force push the old commit id.

grevil’s picture

grevil commented
Status: Needs work » Reviewed & tested by the community

Just moved the phpstan-ignore-line line. Otherwise LGTM! Merging.

grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.