search

perliedman / leaflet-routing-machine

Control for routing in Leaflet
https://www.liedman.net/leaflet-routing-machine/
Other
1.06k stars 347 forks source link

Delete button missing #621

Open tikky opened 3 years ago

tikky commented 3 years ago

Hi there,

on your webpage I am missing a button, where I should click to delete waypoint from routing? delete_missing

I have checked on my phone, and another web browsers and the same problem occurs. So seems to be a bug/problem.

Ruschio commented 3 years ago

Which browser is affected from this problem?

tikky commented 3 years ago

The problem is visible on: Opera, Chrome, Edge - tested on Windows 10 Problem also on iOS @ iPhone11

Works fine only on Firefox

For reference I have tested main page: http://www.liedman.net/leaflet-routing-machine/

Please find 5 screenshots below:

Firefox - here is ok, buttons on right are visible: firefox-ok

Chrome problem - lack of buttons: chrome-problem

Edge the same problem: EDGE-problem

Opera also problem visible: opera-problem

And one more screenshot from mobile iPhone11 with Safari - also buttons are mising: ios-problem

Ruschio commented 3 years ago

Yes I got it on Chrome too but not on Firefox (as you said). I looked at console logs on browsers and seems that on Chrome there are several forbidden requests errors, that there aren't on Firefox. Chrome

But I also tried to implement Routing Machine on my test website and works good with Chrome, so probably there is some browsers problems on that site and they don't get resources in the correct way. Chrome

tikky commented 3 years ago

ok, I will check on my local development env

BTW: on webpage with the problem maybe will be better to use OpenStreetMap tiles - this should resolve the issue

tikky commented 3 years ago

I have checked that on my local development site and I am still experiencing the same issue (tested on Chrome and Opera): image

Here is a full html test, co you can check if you can observe the same:

<!DOCTYPE html>
<html>
<body>

<link rel="stylesheet" href="https://unpkg.com/leaflet@1.7.1/dist/leaflet.css" />
<script src="https://unpkg.com/leaflet@1.7.1/dist/leaflet.js"></script>

<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet-routing-machine/3.2.12/leaflet-routing-machine.css" integrity="sha512-eD3SR/R7bcJ9YJeaUe7KX8u8naADgalpY/oNJ6AHvp1ODHF3iR8V9W4UgU611SD/jI0GsFbijyDBAzSOg+n+iQ==" crossorigin="anonymous" />
<script src="https://cdnjs.cloudflare.com/ajax/libs/leaflet-routing-machine/3.2.12/leaflet-routing-machine.min.js" integrity="sha512-FW2A4pYfHjQKc2ATccIPeCaQpgSQE1pMrEsZqfHNohWKqooGsMYCo3WOJ9ZtZRzikxtMAJft+Kz0Lybli0cbxQ==" crossorigin="anonymous"></script>

<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/perliedman-leaflet-control-geocoder/2.1.0/Control.Geocoder.min.css" integrity="sha512-XK7G+iGsTdf8/PmVD9/ju65AxCIOaUZly1lY8r9jCNrXO5rGzT2lMYCySGEhb/v8XYXybsZUztXvRdGSyxu/3A==" crossorigin="anonymous" />
<script src="https://cdnjs.cloudflare.com/ajax/libs/perliedman-leaflet-control-geocoder/2.1.0/Control.Geocoder.min.js" integrity="sha512-om2lbFibjXmte7C8BK5Pgx2/keKB8Dywi5iRg/7ZPfTFQvGwUlilBflOPRaoj/Nm8UlF9VpzXq2DRI8f2SlBrw==" crossorigin="anonymous"></script>

<h1>Leaflet routing test</h1>

<div style="height: 600px;" id="mapid">
</div>
<script>
var map = L.map('mapid').setView([50.27264, 7.26469], 13);
console.log(map);
L.tileLayer('http://{s}.tile.osm.org/{z}/{x}/{y}.png').addTo(map);
var control = L.Routing.control({
  waypoints: [
    L.latLng(50.67, 17.926),
    L.latLng(50.033136, 20.207316)
  ],
  router: new L.Routing.osrmv1({
    language: 'en',
    profile: 'car'
  }),
  collapsible: true,

  geocoder: L.Control.Geocoder.nominatim({})
}).addTo(map);
</script>
</body>
</html>
Ruschio commented 3 years ago

Ok man, I tried to copy paste your code inside my index page and I got your same issue in Chrome. I opened style editor and the buttons exist, but there aren't showed because you need to add those code at your style:

* { box-sizing: border-box; }

On my previous try I didn't see the issue because of it, so I think adding this code helps to display well all buttons. Try it and write your feedback!

tikky commented 3 years ago

Hello, yes, adding this css seems to resolve an issue. I've checked on Opera/Chrome - and iPhone/iOS So is it a bug or I've done something wrong? For me it is a bug cause even on main page of leaflet-routing-machine did not work as expected.

Anyway thank you for your help and information.

One thing which still looks not correct is a button when you collapse the window. Here is a JSfiddle: https://jsfiddle.net/3f24Ln1x/

and this is how it looks - icon is not centered properly: image

Ruschio commented 3 years ago

Yes I think that the responsive style can be done better than now, but basically you'll need to apply this style:

.leaflet-routing-container, .leaflet-routing-error {
width: 320px;
background-color: white;
padding-top: 4px; <-- remove this
transition: all 0.2s ease;
box-sizing: border-box;
}

.leaflet-routing-container-hide .leaflet-routing-collapse-btn {
display: block;
width: 100%;
height: 100%;
background-image: url('routing-icon.png');
background-repeat: no-repeat;
background-position: center;
}

.leaflet-routing-collapse-btn {
display: table;
margin-right: 10px;
font-size: 24px;
color: #ccc;
font-weight: bold;
margin-left: auto;
}
tikky commented 3 years ago

Hello,

thanx for suggestions. What works for me is to override two css settings but in different place:

<style>
.leaflet-routing-container-hide .leaflet-routing-collapse-btn {
top: 0px;
left: 1px;
}
</style>

Now seems to look fine for me: image

and working code example: https://jsfiddle.net/8ywe7o2z/

Ruschio commented 3 years ago

This is not good when change the screen size, so you will need to remove from padding-top: 4px from the .leaflet-routing-container, .leaflet-routing-error class

And your solution is quite good, but the icon is not perfectly centered, so you will need to use this as i said before:

.leaflet-routing-container-hide .leaflet-routing-collapse-btn {
display: block;
width: 100%;
height: 100%;
background-image: url('routing-icon.png');
background-repeat: no-repeat;
background-position: center;
}
tikky commented 3 years ago

As suggested have deleted padding-top: 4px from the.leaflet-routing-container, .leaflet-routing-error class and changed .leaflet-routing-container-hide .leaflet-routing-collapse-btn as per above, but the icon is not centered: image

Have to add this two lines in bold to make it work for me: .leaflet-routing-container-hide .leaflet-routing-collapse-btn { display: block; width: 100%; height: 100%; background-image: url('routing-icon.png'); background-repeat: no-repeat; background-position: center; / Have to add this two lines: / position: relative; left: 0px; }

Hers is fiddle, if this still not the way as it should be could you please edit the jsfiddle and reply back with correct one? https://jsfiddle.net/bn4kqj3L/2/ (lines 236,237 on css @ jsfiddle)

Ruschio commented 3 years ago

Instead of relative, you should use static and remove left: 0px from the style

tikky commented 3 years ago

Yes, finally with staticand without left: 0pxworks fine: https://jsfiddle.net/z5prxjyw/2/

.leaflet-routing-container-hide .leaflet-routing-collapse-btn {
  display: block;
  width: 100%;
  height: 100%;
  background-image: url('https://raw.githubusercontent.com/perliedman/leaflet-routing-machine/master/dist/routing-icon.png');
  background-repeat: no-repeat;
  background-position: center;
  position: static;
}

So do we need to propose some changes in source files?

Ruschio commented 3 years ago

Maybe yes

tikky commented 3 years ago

So, * { box-sizing: border-box; } should be added to leaflet-routing-machine.css file? Or in different place?