Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Tree: update node size on updateNodes #1281

Open
wants to merge 1 commit into
base: releases/23.2
Choose a base branch
from

Conversation

andibur
Copy link
Member

@andibur andibur commented Nov 27, 2024

The size of a node must be recomputed when the text is changed and if scrollbars are used.

402635

The size of a node must be recomputed when the text is changed and if
scrollbars are used.

402635
@andibur andibur self-assigned this Nov 27, 2024
@andibur andibur requested a review from cguglielmo November 27, 2024 14:33
this._updateNodeSize(nodes);
}

protected _updateNodeSize(nodes: TreeNode[]) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe name it updateNodeSizes (plural) because it takes an array of nodes. Also please move it up to the other update size related methods (updateNodeHeights, _updateNodeDimensions).

tree.render();

let origWidth = node0.width;
let origMaxNodeWidth =tree.maxNodeWidth;
Copy link
Member

Choose a reason for hiding this comment

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

Code does not look properly formatted

@@ -2408,6 +2408,15 @@ export class Tree extends Widget implements TreeModel {
updatedNodes.push(oldNode);
});

// recompute node size if horizontal scrolling is enabled
if (this.isHorizontalScrollingEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

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

I remembered again that updateNodes is probably not the right method to adjust. Unfortunately, there is another method with a similar purpose: changeNode. changeNode should be called if a property changes that affects presentation but not the tree structure (see TreeEvent.TYPE_NODE_CHANGED). So we should add it there (to make it work for Scout Classic as well), but it works a bit differently... Unfortunately, it is not a batch method, instead it uses a microTask to coalesce the update. I tried to implement it here: https://github.com/eclipse-scout/scout.rt/tree/features/cgu/25.1/tree_update_node_size_402635 But it needs some more work.

I think in the future we should combine updateNodes and changeNode because it is confusing.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants