-
-
Notifications
You must be signed in to change notification settings - Fork 335
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
Using ol3-sidebar with OpenLayers v5 #143
Comments
Ok, maybe the problem is that the sidebar is not ready to be used with OL5 right? https://github.com/ThomasG77/sidebar-v2/commit/fc47fbc76d9e473bf01e9d8313760ca6abdb2429 |
I started working on this but I still have issues in traducing the ol3-sidebar to OL version 5. I took inspiration from the official OL Custom Controls example. ol3-sidebar.js
The error I am receiving is:
, which refers to the lines:
|
walkermatt/ol-layerswitcher#78 explains the steps to port the code to OL5. Maybe it can be of help 😸 |
@Turbo87 FYI. I forked your repo and I am working on providing a ol5-sidebar (https://github.com/umbe1987/sidebar-v2/tree/ol5-sidebar). If I succeed, I hope this would be an appreciated enhancement ;) Thanks again for this project BTW! |
I'm almost done!
|
so @umbe1987 is your fork for OpenLayers 5 working? |
@karbecker yes. Indeed, I am actually using it in my own project. The way to sue it is by referencing the built js in the dist folder from the HTML, like:
|
@umbe1987 , I've used your ol5 module. All is fine, except a "lyr is null" error when passing the layerswitcher layers to the sidebar. I recall it was a conversation about it, but cannot find it anymore. Do you recall what is triggering this error? |
@bobiv7 don't know. I am currently using it in my project with ol-layerswitcher and all is fine. Can you elaborate more on the error you're getting? I recall the only bug found after merging was this and I solved it in the latest commit of my branch. Let me know if I can help. Cheers. |
@umbe1987, I applied the example you provided for ol5 (ol5.html), but I see that sidebar extends vertically beyond the map div (from top to bottom of window) regardless of what I set map div height. Is there something (like css) to add ? |
@alsaleem00 Comparing yours and mine ol5.html I see these differences/possible issues: yours: yours: Pay attention that the mine lines refer to the folder structure I have in my fork, so you might want to look at where the files in your project tree actually reside (i.e. ol3-sidebar.css and the built (dist) ol5-sidebar.js). |
But I think the MAIN issue is that you are placing the sidebar div and the map div into two different divs (which is what all the examples in this repo and my fork actually do), and the sidebar is supposed to take 100% the height of its container (the window in your and mine case). I don't know if you try to place the sidebar div INTO the map div maybe you should solve. |
The links in the example are correct. I used the ol5-sidebar.js in the (dist) subfolder. Using the ol5-sidebar.js in the main js did not work. The height problem was solved when placing the sidebar div inside map div AND setting map div to position: relative. (thanks for the hint). In ol5 example, the sidebar div is outside, though. The new issue, the sidebar is placed over the zoom bar when placed to left. I am attaching the example ol5 for your reference. |
Let's not discuss about this issue here please. Just email me if you want (umbertofilippo at tiscali.it). My example is just all the other examples in this repo: the sidebar and the map div are always separated. See also #105. |
@umbe1987 Can you add a note here on how to incorporate your fork into an npm project. Ok, found how to do it here: I would be nice if there was an example of using this in npm. For example do files need to get copied to the dist directory or does that happen automatically with |
as I've mentioned before, the PR in the current state does too much. if we can break it apart and extract just the "support ol5" part from it then I am fine with merging, but I don't want to introduce another bundler for no apparent reason, and definitely not in the same PR. |
Well, actually, as I am using it, I added my fork as a git submodule to my project, and then do The only reason I am doing it is I started using parcel following the tutorials of ol5 on how to build an app (https://openlayers.org/en/latest/doc/tutorials/bundle.html), and there they use When I started I wasn't aware of how bundlers work, so I just created the PR with the aim of changing the code to support ES6 modules (and thus, the way new OL projects are done now). Unfortunately, I did it with the only budler I knew, ignoring that this (justly) would've been a good reason not to merge the PR. Anyway, I think we (I?) might at least save the work I did in js/ol-5sidebar.js and try to bundle it with the bundlers used in this project. I really don't have time these days to work on this, although I'd really love to contribute to this neat project which I use a lot. Hopefully, I'll have more time anytime soon (I am about to have my second baby in days/hours 😍). Meanwhile, feel free of course to ask whatever you need if you feel to keep some of the work I did and port it to another bundler (which anyway has to be done in order to make the new code with ES6 imports work). |
Thanks a lot! @umbe1987 . Your contribution is great! |
@Izzgeo I have been using this and it works fine. in your package.json file in dependencies add:
and then run:
and you should be good to go. |
For me not working: Anyway is there are possibility to have sidebar-v2 to be imported in seperated .js file? |
I've been looking at using ol3-sidebar with Openlayers v6 and have had some success transforming the existing In order to create
npm i --save-dev rollup @rollup/plugin-inject @rollup/plugin-legacy rollup-plugin-re
import inject from '@rollup/plugin-inject';
import replace from 'rollup-plugin-re';
import legacy from '@rollup/plugin-legacy';
export default {
input: 'js/ol3-sidebar.js',
output: {
file: './js/ol-sidebar.mjs',
format: 'es',
},
plugins: [
replace({
patterns: [
// Initial declaration
{
test: 'ol.control.Sidebar = ',
replace: 'var Sidebar = ',
},
// Instance properties
{
test: 'ol.control.Sidebar.prototype',
replace: 'Sidebar.prototype',
},
// Inheritance - reference to constructor
{
test: 'ol.control.Sidebar;',
replace: 'Sidebar;',
},
// Remove dotted namespace to allow inject to replace
// Control occurances
{
test: 'ol\.control\.Control',
replace: 'Control',
},
],
}),
// Adds import statement
inject({
'Control': 'ol/control/Control',
}),
// Adds default export statement
legacy({ 'js/ol3-sidebar.js': 'Sidebar' }),
],
};
npx rollup --config The transformed file doesn't include any ES6 syntax apart from importing and exporting as per ESM. To import the sidebar in an app that uses modules you would install sidebar-v2 via npm then import import Sidebar from 'sidebar-v2/js/ol3-sidebar.mjs';
var sidebar = new Sidebar({
element: 'sidebar',
position: 'left',
}); |
@walkermatt do you have any example anywhere? I was trying to achieve this in my angular app, but failed. |
@MaticDiba here is an example import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import { terser } from 'rollup-plugin-terser';
// Required for ol3-sidebar
import inject from '@rollup/plugin-inject';
import replace from 'rollup-plugin-re';
import legacy from '@rollup/plugin-legacy';
// Transform ol3-sidebar.js into ESM format compatible with ol v6
var sidebarPlugins = [
replace({
include: './node_modules/sidebar-v2/js/ol3-sidebar.js',
patterns: [
// Initial declaration
{
test: 'ol.control.Sidebar = ',
replace: 'var Sidebar = ',
},
// Instance properties
{
test: 'ol.control.Sidebar.prototype',
replace: 'Sidebar.prototype',
},
// Inheritance - reference to constructor (required for versions > 0.4.0)
{
test: 'ol.control.Sidebar;',
replace: 'Sidebar;',
},
// Remove dotted namespace to allow inject to replace
// Control occurances
{
test: 'ol\.control\.Control',
replace: 'Control',
},
// Replace ol.inherits which no longer exists in ol v6
{
test: 'ol\.inherits(ol\.control\.Sidebar, ol\.control\.Control);',
replace: 'Sidebar.prototype = Object.create(Control.prototype);\nSidebar.prototype.constructor = Sidebar;'
}
],
}),
inject({
'Control': 'ol/control/Control',
include: './node_modules/sidebar-v2/js/ol3-sidebar.js'
}),
legacy({ './node_modules/sidebar-v2/js/ol3-sidebar.js': 'Sidebar' }),
];
var config = [
{
input: './map.js',
output: {
name: 'app',
file: './website/build/map.js',
format: 'umd'
},
plugins: [
...sidebarPlugins,
resolve(),
commonjs()
]
}
];
export default config; |
Hi,
I am trying to use the sidebar (v 0.40) as a npm package.
I installed it with
npm install --save sidebar-v2
.After some issues I could get the application to render the sidebar correctly, but still the code (the js) is not working.
The build succeeds, but all i get from console is a
ReferenceError: ol is not defined
message.It refers to the first line of
ol3-sidebar.js
file.To my (still limited) understanding, the ol3-sidebar is not ready to be used as a module, as it has no
export
declaration in the code.Also, to be able to import it, in my
main.js
file I didimport Sidebar from './node_modules/sidebar-v2/js/ol3-sidebar.js';
.Moreover, it should include the import of the necessary ol modules, (probably just a
import Control from 'ol/control/Control';
should be fine?).Correct me if I'm wrong or i' missing something, otherwise I'll be happy to possibly contribute to the project.
Thanks.
The text was updated successfully, but these errors were encountered: