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

Refactor Popup/Tooltip to use common code from DivOverlay #6639

Merged
merged 3 commits into from Jan 19, 2022

Conversation

johnd0e
Copy link
Collaborator

@johnd0e johnd0e commented May 6, 2019

This is continuation of #6613.

(Each commit is self-sufficient and can be reviewed independently)

Edit: rebased on #7531.

  • internal changes
  • DivOverlay: new public api:
     // @method openOn(map: Map): this
     // Adds the overlay to the map.
     // Alternative to `map.openPopup(popup)`/`.openTooltip(tooltip)`.
    
     // @method close(): this
     // Closes the overlay.
     // Alternative to `map.closePopup(popup)`/`.closeTooltip(tooltip)`
     // and `layer.closePopup()`/`.closeTooltip()`.
    
     // @method toggle(layer?: Layer): this
     // Opens or closes the overlay bound to layer depending on its current state.
     // Argument may be omitted only for overlay bound to layer.
     // Alternative to `layer.togglePopup()`/`.toggleTooltip()`.
    
    Notes: openOn used to be Popup-only method, close was private method.

Todo: consider johnd0e/Leaflet@refactor-open-popup-tooltip...johnd0e:refactor-open-popup-tooltip-bind


This change is Reviewable

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

👍

@johnd0e
Copy link
Collaborator Author

johnd0e commented Mar 30, 2021

[...] or use Map.addLayer to open as many as you want.

This is not covered by tests.

Also: layer.togglePopup/Tooltip are not covered.

@johnd0e johnd0e force-pushed the refactor-open-popup-tooltip branch 4 times, most recently from 0b9ced6 to 0dafdad Compare March 31, 2021 15:47
@johnd0e johnd0e changed the title Refactor openPopup/openTooltip to use common code from DivOverlay.js Refactor Popup/Tooltip to use common code from DivOverlay Mar 31, 2021
@johnd0e johnd0e force-pushed the refactor-open-popup-tooltip branch 10 times, most recently from 4d1ae09 to 36de22c Compare April 4, 2021 10:22
@Falke-Design Falke-Design added this to the 1.8 milestone Nov 26, 2021
@mourner
Copy link
Member

mourner commented Dec 20, 2021

@johnd0e can you resolve the conflict in Tooltip.js?

johndoe added 2 commits December 21, 2021 10:05
Refactor [Map\Layer] open/close-Popup/Tooltip to use DivOverlay's openOn/close
Refactor Layer\ togglePopup/Tooltip: use common method DivOverlay\toggle

Popup: do not clear map._popup on close as it's redundant
@mourner mourner merged commit fb0b2a1 into Leaflet:master Jan 19, 2022
Refactor DivOverlay/Popup/Tooltip automation moved this from In progress to Done Jan 19, 2022
JLyne pushed a commit to JLyne/Leaflet that referenced this pull request Jan 21, 2022
* Refactor openPopup/Tooltip to use common functions [Map/Layer]\_initOverlay

* DivOverlay: common functions openOn/close

Refactor [Map\Layer] open/close-Popup/Tooltip to use DivOverlay's openOn/close

* DivOverlay: new public method `toggle`

Refactor Layer\ togglePopup/Tooltip: use common method DivOverlay\toggle

Popup: do not clear map._popup on close as it's redundant
JLyne pushed a commit to JLyne/Leaflet that referenced this pull request Feb 6, 2022
* Refactor openPopup/Tooltip to use common functions [Map/Layer]\_initOverlay

* DivOverlay: common functions openOn/close

Refactor [Map\Layer] open/close-Popup/Tooltip to use DivOverlay's openOn/close

* DivOverlay: new public method `toggle`

Refactor Layer\ togglePopup/Tooltip: use common method DivOverlay\toggle

Popup: do not clear map._popup on close as it's redundant
brunob added a commit to geodiversite/geodiversite that referenced this pull request Aug 2, 2022
brunob added a commit to geodiversite/geodiversite that referenced this pull request Aug 2, 2022
brunob added a commit to geodiversite/geodiversite that referenced this pull request Aug 2, 2022
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.

None yet

3 participants