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: Translate FlashMessage if asset is still in use in media browser inspector #5133

Merged

Conversation

KommunikativCh
Copy link
Collaborator

Bugfix for #5085 no error if asset still in use and show flashmessage solution set class neos-notification-container as id

In media browser inspector if assets are deleted wich are still used no FlashMessage was showed and on second delete an error pages was showed.

Solution:
In Neos.Media:Browser/Resources/Private/Layouts/Default.html change class neos-notification-container to id neos-notification-container, because Notification.js / Toast is searching for id neos-notification-container to insert FlashMessages.

<div class="neos-media-content{f:if(condition: '{tags -> f:count()} > 25', then: ' neos-media-aside-condensed')}"> <div class="neos-media-assets"> <div **id="neos-notification-container"**> <f:render partial="FlashMessages"/> </div> <f:render section="Content"/> </div> <aside class="neos-media-aside">

related: #5085

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

Bugfix for neos#5085 no error if asset still in use and show flashmessage solution set class neos-notification-container as id
@KommunikativCh KommunikativCh changed the title BUGFIX: show FlashMessage if asset is still in use in media browser inspector an no error BUGFIX: show FlashMessage if asset is still in use in media browser inspector and no error Jun 5, 2024
Bugfix for neos#5085 load BackendUserTranslationTrait initializeObject with aliasing initializeObject, initializeObject was overridden by initializeObject from AssetController and and therefore not called.
@KommunikativCh
Copy link
Collaborator Author

During testing it was noticed that the message text was always displayed in the default language. this has now been fixed with the commit.

@dlubitz dlubitz self-requested a review August 13, 2024 12:41
@dlubitz
Copy link
Contributor

dlubitz commented Aug 13, 2024

Thanks @KommunikativCh for this PR. But I couldn't reproduce the error in the issue at all (see #5085).

As this still seems to be a valid point and the initializeObject get's overriden by the class, shall we merge it anyways? What do you think?

See also: https://3v4l.org/0dUtS

Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

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

Ok, now I could reproduce your observation, with the untranslated error message! Makes sence now!

@dlubitz dlubitz changed the title BUGFIX: show FlashMessage if asset is still in use in media browser inspector and no error BUGFIX: Translate FlashMessage if asset is still in use in media browser inspector Aug 13, 2024
Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

I don't like Traits. And I especially don't like Traits that need to be initialized. And I espeically don't like Traits that use initializeObject to be initialized – But that all got nothing to do with your fix ofc.
So if that's the way it works +1 by reading and thanks for your contribution! :)

@dlubitz
Copy link
Contributor

dlubitz commented Aug 16, 2024

I don't like Traits. And I especially don't like Traits that need to be initialized. And I espeically don't like Traits that use initializeObject to be initialized – But that all got nothing to do with your fix ofc. So if that's the way it works +1 by reading and thanks for your contribution! :)

Totally agree. This case is a very good example, how traits shouldn't be used 🤷‍♂️

@dlubitz dlubitz merged commit b84f406 into neos:8.3 Aug 16, 2024
7 checks passed
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