OpenSeaMap / online_chart

OpenSeaMap fullscreen browser chart.
http://map.openseamap.org/
62 stars 32 forks source link

Update to ol@7 #172

Closed oterral closed 1 year ago

oterral commented 1 year ago

This PR contains a massive update of openlayers v7.

index.php and weather.php have been updated . All layers works as before. Trip planner tool and permalink also.

Overview map has been removed, because it is pretty useless and it didn 't work on original website.

Ready for heavy testing.

stevo01 commented 1 year ago

Hello, I startet a test instance for a review of your changes. url:https://test1-alpha.openseamap.org.

note: I got following error from webbrowser when I try to open the URL: Access to image at 'https://t2.openseamap.org/tile/9/271/163.png' from origin 'https://test1-alpha.openseamap.org' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. t2.openseamap.org/tile/9/271/163.png:1 Failed to load resource: net::ERR_FAILED

oterral commented 1 year ago

@stevo01 Thx. But the urls you gave me, it doesn't seem to have the code of this pull request. It's still the current master branch.

It should look like this, see zoom buttons ad zoom bar : image

stevo01 commented 1 year ago

@oterral : The webserver serving https://test1-alpha.openseamap.org uses the following repository and commit ID:

bash-4.3# git remote -v
origin  https://github.com/oterral/online_chart.git (fetch)
origin  https://github.com/oterral/online_chart.git (push)
bash-4.3# git status
HEAD detached at 5c5b025
nothing to commit, working tree clean

and I assume thats the version that you used for the pull request. Let me know if I mixed up something.

oterral commented 1 year ago

@stevo01 Repository and commit number seems good . But it s not this code that is deployed on test1-alpha.

stevo01 commented 1 year ago

@oterral : After bugfix in dockerfile your version is visible. https://github.com/oterral/online_chart.git / commit 5c5b025

note: beside that I changed the config file "online_chart.ini" for usage of OpenStreatMap Tileserver (https://tile.openstreetmap.org/). My webbrowser shows still a CORS error when OpenSeaMap tileserver is used (https://t2.openseamap.org). Can you crosscheck that on your testsystem?

oterral commented 1 year ago

@stevo01 For the CORS issue , the tile backend must send the image with the following header in the response:

access-control-allow-methods: GET, OPTIONS access-control-allow-origin: *

These headera are there when using https://tile.openstreetmap.org/ and https://tiles.openseamap.org/seamark but not when using https://t2.openseamap.org/

stevo01 commented 1 year ago

@stevo01 For the CORS issue , the tile backend must send the image with the following header in the response:

access-control-allow-methods: GET, OPTIONS access-control-allow-origin: *

These headera are there when using https://tile.openstreetmap.org/ and https://tiles.openseamap.org/seamark but not when using https://t2.openseamap.org/

Thanks for the feedback. I will check it.

stevo01 commented 1 year ago
stevo01 commented 1 year ago

@oterral: I added parameter "crossOrigin: null" in sources from test server to fix the issue with cors. I assume this disables the CORS feature. Can you take over that setting in your pull request? From my point of view we can decide later if CORS protection is needed and how to realise it on tile server.

image

wschildbach commented 1 year ago

I started reviewing the code, but since I don't have a laptop available at the moment I stopped again for now... will take up again end of the week. Is this ready for review or still WIP?

oterral commented 1 year ago

@wschildbach maybe it's better to review now before I remove/move some code and files. After, it will be difficult to see what has changed.

wschildbach commented 1 year ago

I've been through the code once now. Great you were able to do a lot of cleanup and refactoring of the code! Thank you a million, this really makes the code base better and more stable.

I left some minor remarks for help texts and such. The compass rose has a bug (see my comment there with the culprit) and the gebco ("Marine profile") layer does not work (see again my comment there).

oterral commented 1 year ago

@wschildbach All is fixed . Thanks again for the review. You can check the 4 last commit to see what I have changed