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

tooltipAnchor has limited or no effect for some Tooltip directions #6764

Closed
5 tasks done
Istador opened this issue Aug 11, 2019 · 6 comments
Closed
5 tasks done

tooltipAnchor has limited or no effect for some Tooltip directions #6764

Istador opened this issue Aug 11, 2019 · 6 comments

Comments

@Istador
Copy link
Contributor

Istador commented Aug 11, 2019

  • I've looked at the documentation to make sure the behavior is documented and expected

The coordinates of the point from which tooltips will "open", relative to the icon anchor.

  • I'm sure this is a Leaflet code issue, not an issue with my own code nor with the framework I'm using (Cordova, Ionic, Angular, React…)
  • I've searched through the issues to make sure it's not yet reported

Steps to reproduce
Steps to reproduce the behavior:

  • Define an Icon with the tooltipAnchor option with a non-zero value for x.
  • Create a Marker using that Icon, that has a Tooltip bound with an top, bottom or center direction.

Expected behavior
The anchor of the Tooltip should move relative to the iconAnchor according to the values defined in the tooltipAnchor. It should have the same behavior as popupAnchor according to the documentation, e.g. if given the same values it should position itself at the same position as the Popup.

Current behavior
tooltipAnchor.x has no effect for the directions top, bottom and center.
tooltipAnchor.y has no effect for the bottom direction.

Environment

  • Leaflet version: 1.5.1
  • Browser (with version): Chrome 76.0.3809.100, Waterfox 56.2.12
  • OS/Platform (with version): Windows 7 (6.1 Build 7601), Ubuntu 18.04

Additional context
Source code responsible for the issue:

if (direction === 'top') {
pos = pos.add(toPoint(-tooltipWidth / 2 + offset.x, -tooltipHeight + offset.y + anchor.y, true));
} else if (direction === 'bottom') {
pos = pos.subtract(toPoint(tooltipWidth / 2 - offset.x, -offset.y, true));
} else if (direction === 'center') {
pos = pos.subtract(toPoint(tooltipWidth / 2 + offset.x, tooltipHeight / 2 - anchor.y + offset.y, true));
} else if (direction === 'right' || direction === 'auto' && tooltipPoint.x < centerPoint.x) {
direction = 'right';
pos = pos.add(toPoint(offset.x + anchor.x, anchor.y - tooltipHeight / 2 + offset.y, true));
} else {
direction = 'left';
pos = pos.subtract(toPoint(tooltipWidth + anchor.x - offset.x, tooltipHeight / 2 - anchor.y - offset.y, true));
}

Minimal example reproducing the issue
https://jsfiddle.net/590jt8fx/

const map = L.map('map').setView([0.0, 0.0], 12)

const icon = L.icon({
  iconUrl: 'https://leafletjs.com/docs/images/logo.png',
  iconSize: [120, 31.8], // (600 x 159)/5
  iconAnchor: [60, 15.9], // center
  popupAnchor: [50, -15.9], // top right
  tooltipAnchor: [50, -15.9], // top right
})

L.marker([0, 0], { icon } )
  .bindPopup("popup is working correctly")
  .bindTooltip("tooltipAnchor.x has no effect", { direction: 'top' })
  .addTo(map)
  
L.marker([-0.02, 0], { icon } )
  .bindTooltip("offset is needed", { direction: 'top', offset: [50, 0] })
  .addTo(map)
  • this example is as simple as possible
  • this example does not rely on any third party code
@vangenechtenbert
Copy link

same here

@johnd0e
Copy link
Collaborator

johnd0e commented May 19, 2020

@Istador
@vangenechtenbert

Is it fixable by #6116?

@Istador
Copy link
Contributor Author

Istador commented May 20, 2020

@Istador
@vangenechtenbert

Is it fixable by #6116?

No, it's even more broken than before.

That's likely because that pull request uses anchor.x for the y axis for the top and bottom direction, instead of for the x axis...


For comparison, this is the current behavior of my minimal example back then when I created the issue. The most recent master branch version shows the same behavior.

Hovering the first image:
Hovering the first image

Hovering the second image:
Hovering the second image


And this is the behavior of the latest commit 20784d7 from pull request #6116 with my minimal example code.

Hovering the first image:
Hovering the first image

Hovering the second image:
Hovering the second image

@johnd0e
Copy link
Collaborator

johnd0e commented May 21, 2020

Can you provide own PR to fix this issue?

mourner pushed a commit that referenced this issue May 24, 2020
This commit fixes issue #6764, see the discussion examples there.
It's an alternative implementation for pull request #6116.

It makes `tooltipAnchor` behave the same like `popupAnchor` regardless of the tooltip's direction. With one exception: the `auto` direction flips the `x` axis of `tooltipAnchor` and the tooltip's `offset` when it switches to `left` depending on the position on the screen. `auto` therefore assumes that the icon is somewhat symmetrical to the left and the right of the y axis, which is the case for the default icon.
Istador added a commit to Istador/pine-interactive-map that referenced this issue Feb 13, 2021
`webpack` v4 => v5:
- `script-ext-html-webpack-plugin` is deprecated and unnecessary now,
the feature I use is now in `html-webpack-plugin`
- `preload-webpack-plugin` is deprecated, use fork
`@vue/preload-webpack-plugin` instead
- `optimize-css-assets-webpack-plugin` wont adjust to `webpack` v5, use
`css-minimizer-webpack-plugin` instead

`leaflet` 1.6.0 => 1.7.1:
- contains my fix for Leaflet/Leaflet#6764
now, so I can remove my workaround.

`sass`:
- `node-sass` is deprecated, use `sass` (`dart-sass`) instead.
- `sass-loader`: the `~` import no longer works. Therefore prefix
spritesmith filenames with `img-`, to fix the ambiguity between
`src/css/icons.scss` and `build/img/icons.scss`

Can't upgrade `wtf_wikipedia` to 8.0.0 or higher right now, because of
issues with:
- https://github.com/substack/https-browserify/issues/9
- jhiesey/stream-http#116
@johnd0e
Copy link
Collaborator

johnd0e commented Apr 5, 2021

@Istador
Isn't this issue already fixed by #7155?

@johnd0e johnd0e closed this as completed Apr 5, 2021
@Istador
Copy link
Contributor Author

Istador commented Apr 5, 2021

Yes, you're correct. I wrongly assumed my commit 5b30f7d would auto close this issue when being merged (I wrote too much in the first sentence of the commit message).

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

No branches or pull requests

3 participants