Skip to content

Comments

XWIKI-23719: The tree finder uses a hard-coded icon#5168

Open
Sereza7 wants to merge 2 commits intoxwiki:masterfrom
Sereza7:XWIKI-23719
Open

XWIKI-23719: The tree finder uses a hard-coded icon#5168
Sereza7 wants to merge 2 commits intoxwiki:masterfrom
Sereza7:XWIKI-23719

Conversation

@Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Feb 10, 2026

Jira URL

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

Changes

Description

  • Updated the style to replace the hard-coded non-semantic icon with a standard one.
  • Changed the webjar url so that it would be evaluated on load. In javascript we still rely on the velocity API to provide standard icons.
  • Updated the styles so that the changes in DOM do not affect presentation too much.

Clarification

  • I didn't think much about the gap value for the flex. Usually .5em works well enough. We don't have a spacing policy yet so I figured it was okay.

Screenshots & Video

After the changes proposed in the PR were applied:
Screenshot From 2026-02-10 17-17-35

Executed Tests

Built the changes successfully with mvn clean install -f xwiki-platform-core/xwiki-platform-tree/xwiki-platform-tree-webjar -Pquality .
Manual tests on my local instance (see screenshot above).
AFAICS there's no test covering the specific DOM changed: https://github.com/search?q=org%3Axwiki%20xtree-finder&type=code

In any case, the icon itself was non interactive and didn't have a node so it's very unlikely tests relied on it. What could break some selectors is that we nested the .xtree-finder input in a new DIV .xtree-finder-container.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • 17.10.X, accessibility bug backport. The scope is quite limited so I believe it wouldn't impact stability much.

* Updated the style to replace the hard-coded non-semantic icon with a standard one.
* Changed the webjar url so that it would be evaluated on load. In javascript we still rely on the velocity API to provide standard icons.
* Updated the styles so that the changes in DOM do not affect presentation too much.
#foreach ($name in $iconNames)
#set ($discard = $icons.put($name, $services.icon.renderHTML($name)))
#end
#[[*/
Copy link
Member

Choose a reason for hiding this comment

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

I prefer we use the REST API. See https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwiki-platform-attachment/xwiki-platform-attachment-api/src/main/resources/js/attachment/move.js#L35 for an example. We're missing a utility on the client side to simplify this (if you have time to add it, it would be awesome), but I think it's still better than mixing Velocity with JavaScript.

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.

2 participants