Skip to content
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

Fix an incorrect calculation of how to shift a mrtree layout. #1060

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

Eddykasp
Copy link
Contributor

@Eddykasp Eddykasp commented Aug 2, 2024

The padding is now correctly taken from the parent not the first child and only the left and top paddings are used to compute the offset.

fixes #1059 and kieler/elkjs#294

The padding is now correctly taken from the parent not the first child
and only the left and top paddings are used to compute the offset.
@Eddykasp Eddykasp added bug Erroneous behaviour. alg-mrtree Affects the ELK Mr. Tree algorithm. labels Aug 2, 2024
@Eddykasp Eddykasp added this to the Release 0.9.2 milestone Aug 2, 2024
@Eddykasp Eddykasp requested a review from soerendomroes August 2, 2024 11:26
Copy link
Contributor

@soerendomroes soerendomroes left a comment

Choose a reason for hiding this comment

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

Maybe we should also add a very simple correctness test for this to ensure that we know once we break this again.

Copy link
Contributor

@soerendomroes soerendomroes left a comment

Choose a reason for hiding this comment

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

Maybe add more non-default values to the test and different values for the different paddings to also test that we use the correct ones.

@Eddykasp Eddykasp merged commit e6e37e5 into eclipse-elk:master Aug 29, 2024
7 checks passed
@Eddykasp Eddykasp deleted the issue1059-fix branch August 29, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alg-mrtree Affects the ELK Mr. Tree algorithm. bug Erroneous behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MrTree nested in a compound node causes incorrect positioning of the sublayout
2 participants