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

Upgrade JS dependencies #102

Merged
merged 16 commits into from
Aug 12, 2024
Merged

Upgrade JS dependencies #102

merged 16 commits into from
Aug 12, 2024

Conversation

amercader
Copy link
Member

This upgrades all the JS libraries to the latest version.

@EricSoroos I decided to go for a better approach than what we were doing up until now.

  • A package.json file lists the libraries we use (at least those which have npm packages).
  • We run npm install and then npm run update-libs. This copies just the distribution files to our js/vendor file, each library to its own folder.

The following libraries are managed manually as they don't have npm packages:

  • shp2geojson
  • ol-helpers

spin.js==2.3.2 and jszip==2.6.0 need to be pinned to old versions otherwise we need to upgrade shp2geojson (which seems unmaintained)

ol-helpers is a difficult one. If you are planning on using it perhaps it would make sense to publish it properly as an npm package, or alternatively we could port the bits we need to this extension

Define packags needed in package.json, add a command to copy the
distribution files to the js/vendor/ folder in gulpfile.js and then
it's just a matter of doing:

1. Updating version numbers in `package.json`
2. Running `npm run update-libs`
But just the distribution files
spin.js==2.3.2 and jszip==2.6.0 need to be pinned to these versions
otherwise we need to upgrade shp2geojson (which seems unmaintained)
Just use a bash script to copy the distribution files from the node
modules folder to the vendor one, which was essetially what gulp was
doing. Store all files at the library root level (no dist/ or src/
folders)
@amercader
Copy link
Member Author

@u10313335 @EricSoroos it would be great if you could tests if these updates break something on your end.

@amercader
Copy link
Member Author

@EricSoroos forgot to say that I incorporated the Derilinx fork of ol-helpers (ol9 branch) so all your changes in #101 are incorporated here

@amercader amercader mentioned this pull request Jul 11, 2024
@u10313335
Copy link

Thanks @amercader. I will test the Shapefile viewer in the following weeks.

My colleague creates the shp2geojson. But unfortunately he had left our team many years ago. Hence, I would recommend to pin the dependencies to old versions.

@u10313335
Copy link

Hi @amercader, I have tested the Shapefile and WMTS viewer.

  1. Shapefile viewer: LGTM at this moment. Also, the shp2geojson is being actively developed again to support the latest JSZip and prepare for a npm package (a big thanks to an intern and my colleague). I will send another patch (or pull request) for the Shapefile view when the new version of shp2geojson is ready (maybe later this month but I cannot guarantee that).
  2. WMTS viewer: I made some small fixes to align with the latest Leaflet. And here is a patch file, which can be applied via git am. After patching, the WMTS viewer works fine:
From 3c89e7a2cff059b4c29fadb30570d62474a4d7df Mon Sep 17 00:00:00 2001
From: Sol Lee <u103133.u103135@gmail.com>
Date: Wed, 7 Aug 2024 09:37:00 +0000
Subject: [PATCH 1/2] Fix filters in webassets

---
 ckanext/geoview/public/webassets.yml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ckanext/geoview/public/webassets.yml b/ckanext/geoview/public/webassets.yml
index 5942d92..a1a512c 100644
--- a/ckanext/geoview/public/webassets.yml
+++ b/ckanext/geoview/public/webassets.yml
@@ -43,7 +43,7 @@ geojson_css:
     - css/geojson_preview.css
 
 wmts_js:
-  filter: rjsmin
+  filters: rjsmin
   output: ckanext-geoview/%(version)s_wmts.js
   extra:
     preload:
@@ -55,13 +55,14 @@ wmts_js:
     - js/common_map.js
     - js/wmts_preview.js
 wmts_css:
+  filters: cssrewrite
   output: ckanext-geoview/%(version)s_wmts.css
   contents:
     - js/vendor/leaflet/leaflet.css
     - css/wmts_preview.css
 
 shp_js:
-  filter: rjsmin
+  filters: rjsmin
   output: ckanext-geoview/%(version)s_shp.js
   extra:
     preload:
-- 
2.30.2


From 55f6ec721c093d8e2164d0dbd0965467c0775259 Mon Sep 17 00:00:00 2001
From: Sol Lee <u103133.u103135@gmail.com>
Date: Thu, 8 Aug 2024 03:47:57 +0000
Subject: [PATCH 2/2] Adjustments for WMTS viewer to align with the latest
 Leaflet

---
 ckanext/geoview/public/css/wmts_preview.css | 32 ++++-----------------
 ckanext/geoview/public/js/wmts_preview.js   |  2 +-
 2 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/ckanext/geoview/public/css/wmts_preview.css b/ckanext/geoview/public/css/wmts_preview.css
index 6d32240..8afc226 100644
--- a/ckanext/geoview/public/css/wmts_preview.css
+++ b/ckanext/geoview/public/css/wmts_preview.css
@@ -11,42 +11,20 @@ html, body {
   height: 100%;
 }
 
-#map .table{
-  width: 300px;
-}
-
-#map label {
-  font-weight: normal;
-}
-
-#map label:after {
-  content: none;
-}
-
-#map input {
-  width: auto;
-  top: auto;
-}
-
 .leaflet-control-layers {
-  box-shadow: 0 1px 7px rgba(0,0,0,0.4);
-  background: #f8f8f9;
-  -webkit-border-radius: 5px;
-  border-radius: 5px;
-  overflow: auto;
   max-height: 200px;
+  overflow-y: hidden;
 }
 
 .ui-opacity {
-  box-shadow: 0 0 8px rgba(0,0,0,0.4);
   background: rgba(255, 255, 255, 0.8);
   position: absolute;
-  left: 13px;
-  top: 70px;
+  left: 14px;
+  top: 80px;
   height: 200px;
   width: 22px;
-  border: 1px solid #888;
-  border-radius: 5px;
+  border: 2px solid rgba(0, 0, 0, 0.2);
+  border-radius: 4px;
   z-index: 1000;
  }
 
diff --git a/ckanext/geoview/public/js/wmts_preview.js b/ckanext/geoview/public/js/wmts_preview.js
index 6f8fbe5..fc5a8d9 100644
--- a/ckanext/geoview/public/js/wmts_preview.js
+++ b/ckanext/geoview/public/js/wmts_preview.js
@@ -140,7 +140,7 @@ ckan.module('wmtspreview', function (jQuery, _) {
       L.DomEvent.disableClickPropagation(container);
 
       // Opacity control for desktop
-      if (!L.Browser.touch) {
+      if (!L.Browser.mobile) {
 	var outer = $('<div id="control" class="ui-opacity">');
 	var inner = $('<div id="handle" class="handle">');
 	var start = false;
-- 
2.30.2

@amercader
Copy link
Member Author

Thanks @u10313335 works great, merging now and doing a new release.

@amercader amercader merged commit 28a8716 into master Aug 12, 2024
7 checks passed
@amercader amercader deleted the js-requirements branch August 12, 2024 13:26
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

Successfully merging this pull request may close these issues.

2 participants