Skip to content

fixed LayerList.moveHigher in case of duplicates #18

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

basisbit
Copy link

If the LayerList contained the same layer more then once, moveHigher did not actually move a visible Layer "higher" (making it visible). Using lastIndexOf, the function should work for all cases as desired.

If the LayerList contained the same layer more then once, moveHigher did not actually move a visible Layer "higher" (visible). Using lastIndexOf, the function should work for all cases as desired.
@basisbit basisbit changed the title fixed LayerList.moveHigher in cases of duplicates fixed LayerList.moveHigher in case of duplicates Sep 12, 2016
@pdavidc
Copy link
Contributor

pdavidc commented Sep 12, 2016

Hi basisbit. I'm unclear on the motivation for duplicate layers in the layer list. What are the use cases in which you're encountering duplicate layers?

@basisbit
Copy link
Author

WorldWindJava is an SDK, so there are various cases where for example because of (bad) user configuration or having one non-deselectable layer-duplicate sort of as fallback layer could cause duplicate layers within the LayerList.
Anyways, there is nothing to check for duplicates in the LayerList.add function, so duplicates should be handled correctly.

@pdavidc
Copy link
Contributor

pdavidc commented Sep 12, 2016

We're certainly aware that WWJ is an SDK. It's still unclear to me why, in the cases you mention, this is the right thing to do.

Your point about handling duplicates makes sense, but we like to understand what motivates a change. The cases you describe appear to be problems best handled by the application.

@basisbit
Copy link
Author

basisbit commented Oct 17, 2016

I agree that these cases in many situations should best be handled by the application because usually, there shouldn't be any duplicate layers in the list. But anyways, the API specification does not state that anywhere and also doesn't check for duplicates before adding additional layers. Thus, "this.indexOf(targetLayer)" is not correct here, but should be "this.lastIndexOf(targetLayer)" instead.

For many use cases, the end user may edit the layers configuration in some config file and thus can set duplicates for whatever reason (maybe for easier grouping / legacy reasons / ...).

@pdavidc pdavidc added the request label Dec 9, 2016
@pdavidc pdavidc removed the request label Jun 16, 2017
@wcmatthysen wcmatthysen mentioned this pull request Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants