lukasbach / react-complex-tree

Unopinionated Accessible Tree Component with Multi-Select and Drag-And-Drop
https://rct.lukasbach.com
MIT License
944 stars 74 forks source link

Drag line position is incorrect when giving item-container a margin. #404

Closed ethan-vanderheijden closed 1 month ago

ethan-vanderheijden commented 1 month ago

Describe the bug If you give the item-container element a vertical margin, the drag line position is calculated incorrectly.

To Reproduce Give item-container a vertical margin like so:

.rct-tree-item-title-container {
    margin-top: 2px;
    margin-bottom: 2px;
}

https://github.com/user-attachments/assets/235005fe-efa1-4ed4-bd85-80489d84dd93

Additional context The issue is that it is calculating the item container's height incorrectly: https://github.com/lukasbach/react-complex-tree/blob/596a62492e5e2b70b64bf47ee732b1c161eb3802/packages/core/src/controlledEnvironment/layoutUtils.ts#L3-L8 It uses .offsetHeight, which doesn't take into account margins. The following patch worked for me:

const firstItem = getDocument()?.querySelector(
    `[data-rct-tree="${treeId}"] [data-rct-item-container="true"]`
);
if (firstItem) {
    const style = getComputedStyle(firstItem);
    // note: divide total margin by two
    // when two item containers are adjacent and both have margins, only one margin is rendered
    return firstItem.offsetHeight + (parseFloat(style.marginBottom) + parseFloat(style.marginTop)) / 2;
} else {
    return 5;
}
lukasbach commented 1 month ago

Thanks for the report, I've fixed it in the latest version.

I've changed the implementation to just use maximum value of full top and full bottom margin instead of the average of top and bottom margin. Your implementation works fine if top and bottom margin are equal, in which case the new implementation will behave identical. But generally, an DOM element with top margin will have its margin flow into the bottom margin of the preceding element, and for differing margins this behaves inconsistent.

Now, if the bottom margin is larger than the top margin, the top margin is ignored completely since it completely flows into the bottom margin of the preceding item. If the top margin is larger, it extends beyond the bottom margin of the preceding element, and the top margin can thus be ignored.

Again, thank you very much for your report and the sample code, that was very helpful.