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

Overlapping markers cause the wrong item to be selected when using: onItemLongPress() #1122

Closed
openBDM opened this Issue Aug 2, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@openBDM
Copy link

openBDM commented Aug 2, 2018

Issue Type

[x ] Question
[x ] Bug
[ ] Improvement
[ ] Build system related
[ ] Performance
[ ] Documentation

Description and/or steps/code to reproduce the problem

image
When such as shown above markers are set on a map and the following code is used:

ItemizedOverlayWithFocus<OverlayItem> mOverlay = new ItemizedOverlayWithFocus<OverlayItem> 
   (this,items,
                new ItemizedIconOverlay.OnItemGestureListener<OverlayItem>() {
                    @Override
                    public boolean onItemSingleTapUp(final int index, final OverlayItem item) {
                        return true;
                    }
                    @Override
                    public boolean onItemLongPress(final int index, final OverlayItem item) {
                        System.out.println("TWICE");
                    
                        return false;
                    }
                });
        mOverlay.setFocusItemsOnTap(true);

        map.getOverlays().add(mOverlay);

It will open the item in the back (the one not selected/longpressed)

How could I solve this?

Environment

Nexus 5 android emulator

If it's a bug, version(s) of android this affects:

All

Version of osmdroid the issue relates to:

6.0.2

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Aug 2, 2018

I don't know if the "wrong" marker is selected (you prefer the top one, don't you?), but this is how I handle such cases:

  • I use a List<OverlayItem>
  • in ItemizedOverlayWithFocus<>.onItemSingleTapUp, I populate this List and always return false, so that all the OverlayItems are checked, and I will have to check the other overlays afterwards
  • I have a MapEventsReceiver before the ItemizedOverlayWithFocus<> (therefore its onItemSingleTapUp is called after), which checks this list (and clear it afterwards)
  • this way I can handle the 0, 1 or n OverlayItem(s) clicked upon the way I want it (in my app, when there are several clicked items I display a list via an Adapter in order to select the actual item - I may want the hidden ones)
  • you can also do it the way I presume you want it - if there are n items you get the last one (supposedly the last being displayed)

Tell me if it works for you.

@copypasteearth

This comment has been minimized.

Copy link

copypasteearth commented Aug 17, 2018

@monsieurtanuki do you by any chance have a way to do it with OnMarkerClickListener?
haha im just thinking and thinking and I have no idea how to do it how you stated above
any way to do it with markers?
and android studio is telling me that MapEventsOverlay which takes MapEventsReceiver is depreciated

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Aug 17, 2018

@copypasteearth I don't have access to my laptop now, but the logic should be the same with markers. I'll create a demo for both by the end of August.

@monsieurtanuki monsieurtanuki self-assigned this Aug 17, 2018

@copypasteearth

This comment has been minimized.

Copy link

copypasteearth commented Aug 17, 2018

ahhh, thanks I got it working how you suggested above but the only thing is that MapEventsOverlay is depreciated. Im just worried about that. Is it ok to still use it and put it into production haha?

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Aug 18, 2018

The MapEventsOverlay class is not deprecated.
Its (Context, MapEventsReceiver) constructor is indeed deprecated, but the other (MapEventsReceiver) constructor isn't.

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Sep 7, 2018

@openBDM @copypasteearth I've just merged new samples in #1150.
Does that give you inspiration to solve your issues?

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Oct 27, 2018

@copypasteearth

This comment has been minimized.

Copy link

copypasteearth commented Oct 27, 2018

Sorry for no responses on my issues. I am in school and i wont get back to my project with maps until late spring.

cbalster pushed a commit to cbalster/osmdroid that referenced this issue Oct 28, 2018

feature/osmdroid#1122 - samples on how to handle a click on overlappi…
…ng items

I created 2 new samples, to be found in "More Samples / Data": "Overlapping ItemizedOverlays' click" and "Overlapping Markers' click"

New classes:
* `SampleItemizedOverlayMultiClick`: Sample on how to handle a click on overlapping `OverlayItem`s, to be found in "More Samples / Data / Overlapping ItemizedOverlays' click"
* `SampleMarkerMultiClick`: Sample on how to handle a click on overlapping `Marker`s, to be found in "More Samples / Data / Overlapping Markers' click"

Impacted classes:
* `GeoPoint`: added a constructor with `IGeoPoint` as parameter
* `SampleFactory`: added the 2 new classes `SampleItemizedOverlayMultiClick` and `SampleMarkerMultiClick`
* `SampleWithMinimapItemizedOverlayWithFocus`: unrelated - used not-deprecated new versions of old methods

cbalster pushed a commit to cbalster/osmdroid that referenced this issue Oct 28, 2018

Merge pull request osmdroid#1150 from osmdroid/feature/osmdroid#1122
feature/osmdroid#1122 - samples on how to handle a click on overlapping items

cbalster pushed a commit to cbalster/osmdroid that referenced this issue Oct 28, 2018

feature/osmdroid#1122 - removed the confusing animation at start of d…
…emos

Impacted classes:
* `SampleItemizedOverlayMultiClick`
* `SampleMarkerMultiClick`

cbalster pushed a commit to cbalster/osmdroid that referenced this issue Oct 28, 2018

Merge pull request osmdroid#1151 from osmdroid/feature/#1122_2
feature/osmdroid#1122 - removed the confusing animation at start of demos
@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Nov 14, 2018

@openBDM I think the fix to your OP would be to edit ItemizedIconOverlay.activateSelectedItems in order to list the items in a reverse order, a bit like what is done in DefaultOverlayManager.onLongPress.
But that doesn't explain why you currently have different results between single tap and long press.
Feel free to PR.

monsieurtanuki added a commit that referenced this issue Feb 4, 2019

bug/#1122 - removed confusion about long press in itemized overlay sa…
…mples

Fixed the return value of `ItemizedIconOverlay.OnItemGestureListener.onItemLongPress` in the samples.
The default expected behavior is that if an item is pressed, the method should return `true`, in order to prevent other items to be potentially selected as well. This is already the case in method `onItemSingleTapUp`.
Of course, that doesn't prevent anyone to consider that `false` is the relevant return value, depending on the purpose of the app. But for the samples, `true` makes more sense.

For all the following classes, those are the changes:
* `onItemLongPress` now returns `true` when the first item is found
* minor refactoring in order to remove editor warnings

Impacted classes:
* `SampleMilitaryIconsItemizedIcons`
* `SampleWithMinimapItemizedOverlayWithFocus`
* `SampleWithMinimapItemizedOverlayWithScale`
* `SampleWithMinimapItemizedoverlay`

monsieurtanuki added a commit that referenced this issue Feb 4, 2019

Merge pull request #1262 from osmdroid/bug/#1122
bug/#1122 - removed confusion about long press in itemized overlay samples
@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

monsieurtanuki commented Feb 4, 2019

Looking back at the issue, I think the fix is only to return true in method onItemLongPress.
Which for some reason was not the case in the samples, and in the OP code excerpt.
I "fixed" the samples of the demo app, assuming that the expected behavior for long clicks is the same as for short clicks: identify only the first item being clicked on.
I've just merged #1262.
Closing the issue now; @openBDM feel free to reopen the issue if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment