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

Change eval methods for multiple feature #1376

Merged
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## UNRELEASED

### Fixed
- Fix `value`, `eval()` and `getLegendData()` for binary operations

## [1.3.1] 2019-06-17

### Fixed
Expand Down
107 changes: 107 additions & 0 deletions debug/advanced/bivariate-legends.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
<!DOCTYPE html>
<html>

<head>
<script src="../../dist/carto-vl.js"></script>
<script src="https://api.tiles.mapbox.com/mapbox-gl-js/v0.52.0/mapbox-gl.js"></script>
<link href="https://api.tiles.mapbox.com/mapbox-gl-js/v0.52.0/mapbox-gl.css" rel="stylesheet" />
<link rel="stylesheet" type="text/css" href="../../examples/style.css">
<style>
ul.legend {
display: grid;
list-style: none;
grid-template-columns: 1fr 1fr 1fr;
grid-template-rows: 1fr 1fr 1fr;
grid-gap: 1px;
width: 100%;
height: 100%;
background: white;
box-sizing: border-box;
margin: 12px 0 0;
padding: 0;
}

ul.legend > li {
height: 70px;
}
</style>
</head>

<body>
<div id="map"></div>
<aside class="toolbox">
<div class="box">
<header>
<h1>Bivariate Legends</h1>
</header>
<section>
<div id="legend" class="bivariate-legend">
<ul id="legend-content" class="legend"></ul>
</div>
<div id="controls">
<ul id="content"></ul>
</div>
</section>
<footer class="js-footer"></footer>
</div>
</aside>
<div id="loader">
<div class="CDB-LoaderIcon CDB-LoaderIcon--big">
<svg class="CDB-LoaderIcon-spinner" viewBox="0 0 50 50">
<circle class="CDB-LoaderIcon-path" cx="25" cy="25" r="20" fill="none"></circle>
</svg>
</div>
</div>

<script>
const map = new mapboxgl.Map({
container: 'map',
style: carto.basemaps.positron,
center: [-96.72033740507385, 32.84383032617839],
zoom: 10.2
});

carto.setDefaultAuth({
username: 'cartovl',
apiKey: 'default_public'
});

const source = new carto.source.Dataset('dallas_mkt');

const viz = new carto.Viz(`
strokeColor: rgba(255,255,255,0.2)
@pop: $pop_sqkm
@percent: $percent_25_29
color: ramp(globalQuantiles($pop_sqkm, 3), [#e8e8e8, #dfb0d6, #be64ac]) *
ramp(globalQuantiles($percent_25_29, 3), [#e8e8e8, #ace4e4, #5ac8c8])
`);

const layer = new carto.Layer('layer', source, viz);
layer.addTo(map);

layer.on('loaded', () => {
hideLoader();

const legends = layer.viz.color.getLegendData();
let content = '';

legends.data.forEach((legend) => {
const colorHex = rgbToHex(legend.value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elenatorro I think we can now remove this rgbToHex thing, and just usa rgba as in feature-and-legend-opacity.html example

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably move this 'debug' example to public ones, what do you think? Or maybe we should do a small blog post?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally! I'd like to have this in a separate task (since this one is a bit big), and it'd be great to have more than one example (but not too much, I mean, like two or three). At leas this one and another one that combines with and color properties.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. We can keep the rgbToHex if it is just a debug example.

content +=
`<li style="background-color:${colorHex};"></li>`
});

document.getElementById('legend-content').innerHTML = content;
});

function hideLoader() {
document.getElementById('loader').style.opacity = '0';
}

function rgbToHex(color) {
return "#" + ((1 << 24) + (color.r << 16) + (color.g << 8) + color.b).toString(16).slice(1);
}
</script>
</body>

</html>
9 changes: 8 additions & 1 deletion src/client/windshaft-filtering.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { And, Or, Equals, NotEquals, LessThan, LessThanOrEqualTo, GreaterThan, GreaterThanOrEqualTo } from '../renderer/viz/expressions/binary';
import GreaterThan from '../renderer/viz/expressions/binary/GreaterThan';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this small refactor here? Was it maybe based on some tooling?

Copy link
Contributor

@VictorVelarde VictorVelarde Jun 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see it now... you deleted the binary.js, much better

import GreaterThanOrEqualTo from '../renderer/viz/expressions/binary/GreaterThanOrEqualTo';
import LessThan from '../renderer/viz/expressions/binary/LessThan';
import LessThanOrEqualTo from '../renderer/viz/expressions/binary/LessThanOrEqualTo';
import And from '../renderer/viz/expressions/binary/And';
import Or from '../renderer/viz/expressions/binary/Or';
import Equals from '../renderer/viz/expressions/binary/Equals';
import NotEquals from '../renderer/viz/expressions/binary/NotEquals';
import { In, Nin } from '../renderer/viz/expressions/belongs';
import Between from '../renderer/viz/expressions/between';
import Property from '../renderer/viz/expressions/basic/property';
Expand Down
13 changes: 12 additions & 1 deletion src/interactivity/featureVizProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ export default class FeatureVizProperty {
}

get value () {
return this._viz[this._propertyName].eval(this._properties);
return this._viz[this._propertyName].value;
}

eval (...properties) {
const props = [];
properties.forEach((property) => {
const prop = {};
prop[property] = this._properties[property];
props.push(prop);
});

return this._viz[this._propertyName].eval(props);
}
}
29 changes: 15 additions & 14 deletions src/renderer/viz/expressions.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,20 +451,21 @@ import { Nin } from './expressions/belongs';

import Between from './expressions/between';

import { Mul } from './expressions/binary';
import { Div } from './expressions/binary';
import { Add } from './expressions/binary';
import { Sub } from './expressions/binary';
import { Mod } from './expressions/binary';
import { Pow } from './expressions/binary';
import { GreaterThan } from './expressions/binary';
import { GreaterThanOrEqualTo } from './expressions/binary';
import { LessThan } from './expressions/binary';
import { LessThanOrEqualTo } from './expressions/binary';
import { Equals } from './expressions/binary';
import { NotEquals } from './expressions/binary';
import { Or } from './expressions/binary';
import { And } from './expressions/binary';
// Binary Operations
import Add from './expressions/binary/Add';
import And from './expressions/binary/And';
import Div from './expressions/binary/Div';
import Equals from './expressions/binary/Equals';
import GreaterThan from './expressions/binary/GreaterThan';
import GreaterThanOrEqualTo from './expressions/binary/GreaterThanOrEqualTo';
import LessThan from './expressions/binary/LessThan';
import LessThanOrEqualTo from './expressions/binary/LessThanOrEqualTo';
import Mod from './expressions/binary/Mod';
import Mul from './expressions/binary/Mul';
import NotEquals from './expressions/binary/NotEquals';
import Or from './expressions/binary/Or';
import Pow from './expressions/binary/Pow';
import Sub from './expressions/binary/Sub';

import Blend from './expressions/blend';

Expand Down
1 change: 1 addition & 0 deletions src/renderer/viz/expressions/Ramp.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export default class Ramp extends BaseExpression {
super({ input, palette });
this.palette = palette;
this.others = others;
this.type = palette.type;
this._defaultOthers = others === DEFAULT_RAMP_OTHERS;
}

Expand Down
1 change: 1 addition & 0 deletions src/renderer/viz/expressions/basic/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export default class List extends Base {

super(elems);
this.elems = elems;
this.type = elems[0].type;
}

get value () {
Expand Down
Loading