Display Bootstrap-like tooltips for menu items #1249
Conversation
|
|
|
I have added the script and link tags in please review my pr. Thanks! |
There was a problem hiding this comment.
OK, I looked back at my original clarification and although we had been editing inside EditAction back then, at that time, that file contained ALL the tools. If you look higher, we were actually editing the Transparency tool (now called Opacity):
Leaflet.DistortableImage/src/edit/DistortableImage.EditToolbar.js
Lines 12 to 17 in 90c9777
So we actually need to be editing the tooltip there, in its new file location:
Notice how most of the actions actually extend EditAction -- EditAction is like the common parent class, and each actual tool is extending it. Does that make sense? So I think the code on line 42 of EditAction is OK. So, let's see if we can get the tooltip text manually...
Yes, I can confirm that the toolbar item shows:
<a class="leaflet-toolbar-icon drag" href="#" role="button" data-bs-toggle="tooltip" data-bs-placement="top" data-bs-title="Drag Image"><svg class="ldi-icon ldi-drag" role="img" focusable="false"><use xlink:href="#drag"></use></svg></a>Good! data-bs-title is present! And, i tried running new bootstrap.Tooltip(document.getElementsByClassName('leaflet-toolbar-icon drag')[0]) and it worked:
So, I wonder if we are failing in line 1 of index.js. Let me dig a bit more.
examples/archive.html
Outdated
| @@ -95,9 +95,6 @@ <h3 id="offcanvasRightLabel">Images</h3> | |||
| form.addEventListener('submit', (event) => { | |||
| event.preventDefault(); | |||
| const url = input.value.replace('details', 'metadata'); | |||
| let splitUrl = url.split('/'); | |||
There was a problem hiding this comment.
First, just noting this seems like an unrelated change -- can you try reverting it with:
cd examples
wget https://raw.githubusercontent.com/publiclab/Leaflet.DistortableImage/main/examples/archive.html
cd ../
git add dist
git commit -m 'removed unrelated changes'
git push
| @@ -1,3 +1,5 @@ | |||
| const tooltipTriggerList = document.querySelectorAll('[data-bs-toggle="top"]') | |||
There was a problem hiding this comment.
So, here is I think the issue. tooltipTriggerList evaluates to an empty NodeList. I think we're running this code too early. If you want to run it AFTER page JS activity is complete, we can put it under the (function() { line. That defers execution until the page is ready. Can we do that? We may need to define it with global scope above, then set it once the page is ready.
There was a problem hiding this comment.
Hi, I tried what you suggested
let tooltipTriggerList;
let tooltipList;
let map;
(function() {
map = L.map('map').setView([51.505, -0.09], 13);
map.addGoogleMutant();
map.whenReady(function() {
tooltipTriggerList = document.querySelectorAll('[data-bs-toggle="tooltip"]')
tooltipList = [...tooltipTriggerList].map(tooltipTriggerEl => new bootstrap.Tooltip(tooltipTriggerEl))
img = L.distortableImageOverlay('example.jpg', {
selected: true,
fullResolutionSrc: 'large.jpg',
}).addTo(map);
});
})();
L.Control.geocoder().addTo(map);
This is how it looks like now. I'm still not able to see the tooltip : (
|
I also want to suggest that we include some documentation. We shouldn't make Bootstrap a required dependency. I think this fails gracefully if it's not present, right? The only code that will fail is still in our examples/index.js file, not in the core library. But we can add a line to the README saying that to enable tooltip formatting, you can include a) Bootstrap 5 and b) the lines of code we're adding to index.js, to load these styles. How does that sound? |
|
Sounds good! I'll start working on it |
Fixes #104 (<=== Add issue number here)
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
grunt testIf tests do fail, click on the red
Xto learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!