Turbo87 / sidebar-v2

A responsive sidebar with tabs for Leaflet, OpenLayers, Google Maps, ...
https://turbo87.github.io/sidebar-v2/examples/
MIT License
849 stars 336 forks source link

Using ol3-sidebar with OpenLayers v5 #143

Open umbe1987 opened 6 years ago

umbe1987 commented 6 years ago

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 did import 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.

umbe1987 commented 6 years ago

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 If so, it would be nice it to be, don't know if it's already in your plans.

umbe1987 commented 6 years ago

I started working on this but I still have issues in traducing the ol3-sidebar to OL version 5. I am testing it using the v0.4.0 package on npm. Please, firts of all, can anybody confirm the sidebar is not compatible with OL5 yet?

I took inspiration from the official OL Custom Controls example. This is the code I've been testing so far (didn't do a branch yet as I don't know this is the real problem):

ol3-sidebar.js

import Control from 'ol/control/Control';
import { inherits } from 'ol/util.js';
import { defaults as defaultControls } from 'ol/control.js';

/**
* Define a namespace for the application.
*/
window.app = {};
var app = window.app;

export default app.Sidebar = function (settings) {

    var defaults = {
        element: null,
        position: 'left'
    }, i, child;

    this._options = Object.assign({}, defaults, settings);

    Control.call(this, {
        element: document.getElementById(this._options.element),
        target: this._options.target
    });

    // Attach .sidebar-left/right class
    this.element.classList.add('sidebar-' + this._options.position);

    // Find sidebar > div.sidebar-content
    for (i = this.element.children.length - 1; i >= 0; i--) {
        child = this.element.children[i];
        if (child.tagName === 'DIV' &&
                child.classList.contains('sidebar-content')) {
            this._container = child;
        }
    }

    // Find sidebar ul.sidebar-tabs > li, sidebar .sidebar-tabs > ul > li
    this._tabitems = this.element.querySelectorAll('ul.sidebar-tabs > li, .sidebar-tabs > ul > li');
    for (i = this._tabitems.length - 1; i >= 0; i--) {
        this._tabitems[i]._sidebar = this;
    }

    // Find sidebar > div.sidebar-content > div.sidebar-pane
    this._panes = [];
    this._closeButtons = [];
    for (i = this._container.children.length - 1; i >= 0; i--) {
        child = this._container.children[i];
        if (child.tagName == 'DIV' &&
                child.classList.contains('sidebar-pane')) {
            this._panes.push(child);

            var closeButtons = child.querySelectorAll('.sidebar-close');
            for (var j = 0, len = closeButtons.length; j < len; j++) {
                this._closeButtons.push(closeButtons[j]);
            }
        }
    }
};

inherits(app.Sidebar, Control);

defaultControls({
    attributionOptions: {
        collapsible: false
    }
}).extend([
    new app.Sidebar
]);

app.Sidebar.prototype.setMap = function(map) {
    var i, child;

    for (i = this._tabitems.length - 1; i >= 0; i--) {
        child = this._tabitems[i];
        var sub = child.querySelector('a');
        if (sub.hasAttribute('href') && sub.getAttribute('href').slice(0,1) == '#') {
            sub.onclick = this._onClick.bind(child);
        }
    }

    for (i = this._closeButtons.length - 1; i >= 0; i--) {
        child = this._closeButtons[i];
        child.onclick = this._onCloseClick.bind(this);
    }
};

app.Sidebar.prototype.open = function(id) {
    var i, child;

    // hide old active contents and show new content
    for (i = this._panes.length - 1; i >= 0; i--) {
        child = this._panes[i];
        if (child.id == id)
            child.classList.add('active');
        else if (child.classList.contains('active'))
            child.classList.remove('active');
    }

    // remove old active highlights and set new highlight
    for (i = this._tabitems.length - 1; i >= 0; i--) {
        child = this._tabitems[i];
        if (child.querySelector('a').hash == '#' + id)
            child.classList.add('active');
        else if (child.classList.contains('active'))
            child.classList.remove('active');
    }

    // open sidebar (if necessary)
    if (this.element.classList.contains('collapsed')) {
        this.element.classList.remove('collapsed');
    }

    return this;
};

app.Sidebar.prototype.close = function() {
    // remove old active highlights
    for (var i = this._tabitems.length - 1; i >= 0; i--) {
        var child = this._tabitems[i];
        if (child.classList.contains('active'))
            child.classList.remove('active');
    }

    // close sidebar
    if (!this.element.classList.contains('collapsed')) {
        this.element.classList.add('collapsed');
    }

    return this;
};

app.Sidebar.prototype._onClick = function() {
    if (this.classList.contains('active')) {
        this._sidebar.close();
    } else if (!this.classList.contains('disabled')) {
        this._sidebar.open(this.querySelector('a').hash.slice(1));
    }
};

app.Sidebar.prototype._onCloseClick = function() {
    this.close();
};

The error I am receiving is:

TypeError: this.element is null

, which refers to the lines:

// Attach .sidebar-left/right class
this.element.classList.add('sidebar-' + this._options.position);
umbe1987 commented 6 years ago

walkermatt/ol-layerswitcher#78 explains the steps to port the code to OL5. Maybe it can be of help 😸

umbe1987 commented 6 years ago

@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 ;) Please if you want let me know if there's anything you want me to take into account before PR.

Thanks again for this project BTW!

umbe1987 commented 6 years ago

I'm almost done! Still need to figure how to rewrite the defaults var but everything seems to point to the right direction! Will work at it asap ;) Looking at my ol5-sidebar.js, if you have any hint how to translate the following lines of code, it'll be easier, but otherwise I'll figure out myself.


    var defaults = {
        element: null,
        position: 'left'
    }, i, child;

this._options = Object.assign({}, defaults, settings);
karkraeg commented 5 years ago

so @umbe1987 is your fork for OpenLayers 5 working?

umbe1987 commented 5 years ago

@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:

/examples/ol5.html

<script src="../dist/ol5-sidebar.js"></script>

ghost commented 5 years ago

@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?

umbe1987 commented 5 years ago

@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.

alsaleem00 commented 5 years ago

@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. That is on FF 63. In IE 11, panels do not slide out, too. ol5.html is attached.

Is there something (like css) to add ?

image ol5.zip

umbe1987 commented 5 years ago

@alsaleem00 Comparing yours and mine ol5.html I see these differences/possible issues:

yours: <link rel="stylesheet" href="ol3-sidebar.css" /> mine: <link rel="stylesheet" href="../css/ol3-sidebar.css" />

yours: <script src="ol5-sidebar.js"></script> mine: <script src="../dist/ol5-sidebar.js"></script>

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).

umbe1987 commented 5 years ago

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.

alsaleem00 commented 5 years ago

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. ol5-example.zip

umbe1987 commented 5 years ago

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.

woodbri commented 5 years ago

@umbe1987 Can you add a note here on how to incorporate your fork into an npm project.

Ok, found how to do it here: https://medium.com/@jonchurch/use-github-branch-as-dependency-in-package-json-5eb609c81f1a

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 import Sidebar from 'js/ol5-sidebar'; what about css files.

woodbri commented 5 years ago

@Turbo87 any chance that @umbe1987 changes can get merged to add support for ol5 in npm.

Turbo87 commented 5 years ago

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.

umbe1987 commented 5 years ago

@woodbri

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 import Sidebar from 'js/ol5-sidebar'; what about css files.

Well, actually, as I am using it, I added my fork as a git submodule to my project, and then do import Sidebar from 'js/ol5-sidebar'; as you said. After that anyway I am bundling everything with parcel and use the built (transpiled) code which I place in a dist folder (dist/ol5-sidebar.js).

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 parcel.

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 :heart_eyes:). 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).

lzzgeo commented 5 years ago

Thanks a lot! @umbe1987 . Your contribution is great! I'm just tring to use OL5 with sidebar-v2. lots of efforts could be saved If it works. Hopefully, PR will be done ASAP. 💯

woodbri commented 5 years ago

@Izzgeo I have been using this and it works fine. in your package.json file in dependencies add:

"sidebar-v2": "umbe1987/sidebar-v2#ol5-sidebar"

and then run:

npm install

and you should be good to go.

JPelda commented 5 years ago

For me not working: SyntaxError: import declarations may only appear at top level of a module ol5-sidebar.js:2

Anyway is there are possibility to have sidebar-v2 to be imported in seperated .js file?

walkermatt commented 4 years ago

I've been looking at using ol3-sidebar with Openlayers v6 and have had some success transforming the existing js/ol3-sidebar.js into ECMAScript module format (ESM) via rollup. I'm wondering if this approach could be used to create js/ol-sidebar.mjs file that could be included in the npm package.

In order to create js/ol-sidebar.mjs the following steps are required:

  1. Install rollup and required plugins:
    npm i --save-dev rollup @rollup/plugin-inject @rollup/plugin-legacy rollup-plugin-re
    1. Create rollup.config.js with the following contents
      
      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' }), ], };

3. Run rollup to create `js/ol-sidebar.mjs`
```sh
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 Sidebar like so:

import Sidebar from 'sidebar-v2/js/ol3-sidebar.mjs';

var sidebar = new Sidebar({
  element: 'sidebar',
  position: 'left',
});
MaticDiba commented 4 years ago

@walkermatt do you have any example anywhere? I was trying to achieve this in my angular app, but failed.

walkermatt commented 4 years ago

@MaticDiba here is an example rollup.config.js based on config I've used in a project that transforms ol3-sidebar during the build:

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;