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 dirty flags when re-ordering layers #1497

Merged
merged 1 commit into from
Dec 31, 2023

Conversation

jschleic
Copy link
Contributor

The index of the top layer in the view starts with zero, while the rank of the layers count backwards. Thus moving the second-last to the last position should set the dirty flag of rank 0 and 1. Instead the former implementation set the dirty flag for layers >= 5, when moving "Red" to the bottom in the test map - resulting in the wrong layers saved.

Fixes #375

The index of the top layer in the view starts with zero, while the rank of
the layers count backwards. Thus moving the second-last to the last
position should set the dirty flag of rank 0 and 1.
Instead the former implementation set the dirty flag for layers >= 19 in
a list of 20 layers - resulting in the wrong layers saved.

Fixes umap-project#375
@yohanboniface yohanboniface merged commit ced7f3d into umap-project:master Dec 31, 2023
12 checks passed
@yohanboniface
Copy link
Member

Sounds good to me. Thanks a lot for spotting this and providing the fix :)

if (e.finalIndex === 0) layer.bringToTop()
else if (e.finalIndex > e.initialIndex) layer.insertBefore(other)
else layer.insertAfter(other)
this.map.eachDataLayerReverse((datalayer) => {
if (datalayer.getRank() >= minIndex) datalayer.isDirty = true
if (datalayer.getRank() >= minIndex && datalayer.getRank() <= maxIndex)
datalayer.isDirty = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not datalayer.isDirty = datalayer.getRank() >= minIndex && datalayer.getRank() <= maxIndex?

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

Successfully merging this pull request may close these issues.

Layer order broke on update
3 participants