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: Fixed table view of asset editor #5432

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Conversation

robinroloff
Copy link
Contributor

Related to #5430

This table view:

image

doesn't currently work because the event listener checks whether there is an or a but doesn't check for . I fixed it by removing the check completely and using e.currentTarget instead of e.target.

The check was not necessary because the eventlistener was already only added to the elements that meet the criteria.
The use of event.currentTarget ensures that you always use the element that the eventlistener was actually added to, whereas event.target is the element that was clicked, which is not necessary in this case.

Nothing has to be adjusted to use this.

@dlubitz
Copy link
Contributor

dlubitz commented Jan 9, 2025

Thank you @robinroloff for this PR. Looks good to me by reading and I'll test this later.

@dlubitz dlubitz self-requested a review January 16, 2025 09:28
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.

I guess I found the reason for the removed early return.

image

You are now not able to open the "..." action menu on the right side, as it immediatelly closes the dialog and selects the image.

@robinroloff
Copy link
Contributor Author

@dlubitz added a new check that fixes this
047f598

@dlubitz dlubitz self-requested a review January 17, 2025 10:00
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.

Great, that works for me now. Thank you

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

👍 by 👀

@dlubitz dlubitz merged commit 3028ea8 into neos:8.3 Jan 17, 2025
2 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