Skip to content

Conversation

@hehoon
Copy link
Collaborator

@hehoon hehoon commented Dec 13, 2025

I have JIRA issue created

  • branch and/or PR name(s) includes JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected
  • FLP integration tests were ran successful

To fix the issue the following has been done:

  • Replace map with flatMap in recursive treeRow traversal

In addition to this:

  • Prevent redundant notify() calls during deep tree operations
  • Perform minor code cleanup for clarity and maintainability

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@hehoon hehoon marked this pull request as ready for review December 15, 2025 12:36
@hehoon hehoon requested a review from graduta as a code owner December 15, 2025 12:36
Copy link
Member

@graduta graduta left a comment

Choose a reason for hiding this comment

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

Very good bug fix!
Looking at branchRow and leafRow, I think these have a lot of duplicated code and we should make one reusable component. What do you think?

@hehoon hehoon requested a review from graduta December 17, 2025 09:58
open ? iconCaretBottom : iconCaretRight,
'',
{
paddingLeft: `${level + 0.25}em`,
Copy link
Member

Choose a reason for hiding this comment

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

You do not need the padding at all now after the refactor.
You also removed the span element, so the default padding of 0.3 from table element is now applied.

Remove the paddingLeft from the call of treeRowElement both places and you will see it looks like before which is what we want

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed, paddingLeft controls the tree indentation and is therefore required.

' ',
tree.name,
h('td.highlight.flex-row.items-center.g1', {
style: styling,
Copy link
Member

Choose a reason for hiding this comment

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

Given that you are not applying any chances, why not naming the variable style and use the short version { style }?

key: path, title: path, onclick: selectItem, class: className, id: path,
key,
id: key,
title: name,
Copy link
Member

Choose a reason for hiding this comment

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

Same here title vs name

@hehoon hehoon requested a review from graduta December 17, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants