-
Notifications
You must be signed in to change notification settings - Fork 14
[OGUI-1661] Object tree breaking when collapsing parents without first collapsing children #3234
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
base: dev
Are you sure you want to change the base?
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this 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?
| open ? iconCaretBottom : iconCaretRight, | ||
| '', | ||
| { | ||
| paddingLeft: `${level + 0.25}em`, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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
I have JIRA issue created
To fix the issue the following has been done:
mapwithflatMapin recursive treeRow traversalIn addition to this:
notify()calls during deep tree operations