Skip to content

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Oct 17, 2025

Jira URL

https://jira.xwiki.org/browse/XWIKI-23573

Changes

Description

  • Took into account the possibility of the targetElement being the icon inside of the button.

Clarifications

  • Somehow with the new DOM the icons can be targets for the two events of the buttons.

Screenshots & Video

None

Executed Tests

Built changes with mvn clean install -f xwiki-platform-core/xwiki-platform-attachment/xwiki-platform-attachment-ui -Pquality
Ran the docker tests: mvn clean install -f xwiki-platform-core/xwiki-platform-attachment/xwiki-platform-attachment-test/xwiki-platform-attachment-test-docker

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • 17.9.X

* Took into account the possibility of the targetElement being the icon inside of the button.
Copy link
Member

@mflorea mflorea left a comment

Choose a reason for hiding this comment

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

I have some suggestions you may want to try out.

Comment on lines +640 to +641
let deleteTool = event.element();
if (!deleteTool.hasClassName('btn')) deleteTool = deleteTool.up('.btn');
Copy link
Member

Choose a reason for hiding this comment

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

I'd try to use the native API:

Suggested change
let deleteTool = event.element();
if (!deleteTool.hasClassName('btn')) deleteTool = deleteTool.up('.btn');
const deleteTool = event.target.closest('.btn');

See https://developer.mozilla.org/en-US/docs/Web/API/Event/target and https://developer.mozilla.org/en-US/docs/Web/API/Element/closest (note that closest starts from the element is being called on).

Comment on lines +684 to +685
let targetElement = event.element();
if (!targetElement.hasClassName('btn')) targetElement = targetElement.up('.btn');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let targetElement = event.element();
if (!targetElement.hasClassName('btn')) targetElement = targetElement.up('.btn');
const targetElement = event.target.closest('.btn');

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.

2 participants