Skip to content

kb fix modifying child aside data instead of just current article - #25396

Open
cconard96 wants to merge 2 commits into
glpi-project:12.0/bugfixesfrom
cconard96:fix/25394
Open

kb fix modifying child aside data instead of just current article#25396
cconard96 wants to merge 2 commits into
glpi-project:12.0/bugfixesfrom
cconard96:fix/25394

Conversation

@cconard96

Copy link
Copy Markdown
Member

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.

Description

Child articles are placed under the parent's li[data-glpi-kb-article-id] element so selectors targeting it would apply to child articles as well unless specifically only looking under the direct .article-line child.

@cconard96 cconard96 self-assigned this Sep 3, 2026

@f2cmb f2cmb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice fix for the child-article bug, but this breaks title/illustration sync for the search results view. .article-line only exists in the tree markup (_article_row.html.twig), not in aside_search_results.html.twig. So if the article you're editing is currently showing up in a search result (aka searching its own title while viewing it), the visible row stays stale after save while the hidden tree row updates fine.

Tested it: renamed a title, saved, and the search result kept the old text.

Might be cleaner to scope tree and search results separately, like AsideController.js:1103 already does for favorites. The search results list is flat so it doesn't need the > .article-line restriction at all.

@cconard96

cconard96 commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Nice fix for the child-article bug, but this breaks title/illustration sync for the search results view. .article-line only exists in the tree markup (_article_row.html.twig), not in aside_search_results.html.twig.

Why though? Wouldn't it of been easier to have the exact same structure for the list HTML and just render each article as a top-level item instead of using its actual position when searching? As well, if you wanted a reactive UI, plain JS was never going to be the cleanest solution for it.

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