Skip to content
This repository has been archived by the owner on Jan 15, 2022. It is now read-only.

ch04_thematic question #2

Closed
WillieMaddox opened this issue May 24, 2016 · 4 comments
Closed

ch04_thematic question #2

WillieMaddox opened this issue May 24, 2016 · 4 comments

Comments

@WillieMaddox
Copy link

Hello,

I was playing around with your example "ch04_thematic" and noticed that the drop down menu displays the properties (e.g. pop_est, name, etc.) correctly with ol v3.11.0 or older, but is empty when using ol v3.12.0 or newer. The problem seems to be related to the buildHeaders function not calling the line, this.set('headers', headers). Or maybe the headers are getting set but the propertychange event isn't firing. I've isolated it down that far, but am running out of debugging ideas.

I compared the ol.layer.Vector documentation between v3.11.0 and v3.12.0. There is no noticeable difference that I can see between the two versions which makes me think it's a problem on my end. I've been at this for hours and still cannot find the source of the error. Are you aware of anything that might be causing this?

Thanks,

@GaborFarkas
Copy link
Owner

Hi,

Just checked this with v3.15.1, and v3.16.0. The headers are set correctly, however, as you guessed it right, the propertychange event isn't firing, thus the problem isn't on your end. No matter, if I set the property with set, or setProperties. An ugly temporal workaround would be setting the updater to listen for every change event.

@WillieMaddox
Copy link
Author

Well, I'm glad I'm not the only one. I was going crazy trying to figure this out. Still, it doesn't make any sense. Thanks for checking.

@WillieMaddox
Copy link
Author

Fixed.
In the buildHeaders method, I set a breakpoint at this.set('headers', headers); and inspected the value of this.getProperties('headers').
To my surprise the headers were already populated. So of course the propertyChange event won't get fired; headers === this.getProperties('headers') returns true.

So I just added the line,
this.setProperties({'headers': {}});
before
this.set('headers', headers);

Checks good with v3.11.0, v3.12.0 and v3.16.0.

If you think of a more appropriate solution, I'd be interested in hearing about it.

@GaborFarkas
Copy link
Owner

Okay, so first of all, thanks for your help with the debugging. Your findings leaded to the solution.

The case is, there is a "new" logic in OL3, which prevents the setters from running, when the new value equals to the old one.

What the old code did:

var headers = this.get('headers') || {};

So what did it grab? The pointer to the headers object, which is stored in the layer. Now, if we look at the updating logic:

for (var i = 0; i < features.length; i += 1) {
    var attributes = features[i].getProperties();
    for (var j in attributes) {
        if (typeof attributes[j] !== 'object' && !(j in headers)) {
            headers[j] = 'string';
        }
    }
}

we can see, that the code updates the headers object in place. Thus, in the end, when OL3 checks, if the stored headers object equals to our updated object, it finds, the two objects are essentially the same, as they are both pointers to the same object, which was updated in place.

The last piece of the puzzle: why the list wasn't populated when buildHeaders was called the first time? Because, a headers object was already provided on layer construction:

    map = new ol.Map({
        target: 'map',
        layers: [
            new ol.layer.Tile({
                source: new ol.source.OSM(),
                name: 'OpenStreetMap'
            }),
            new ol.layer.Vector({
                source: new ol.source.Vector({
                    format: new ol.format.GeoJSON({
                        defaultDataProjection: 'EPSG:4326'
                    }),
                    url: '../../res/world_countries.geojson'
                }),
                name: 'World Countries',
                headers: {
                    pop_est: 'integer',
                    gdp_md_est: 'integer'
                }
            })
        ],

You can see my solution to this issue at 72dd869, I just messed up the issue number.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants