Skip to content

Assign the cluster id to the returned GeoJSON Feature.id property - #94

Merged
asheemmamoowala merged 2 commits into
masterfrom
88-cluster-feature-id
Jul 23, 2018
Merged

Assign the cluster id to the returned GeoJSON Feature.id property#94
asheemmamoowala merged 2 commits into
masterfrom
88-cluster-feature-id

Conversation

@asheemmamoowala

Copy link
Copy Markdown
Contributor

Addresses #88.

cc @mourner @ryanbaumann

@asheemmamoowala
asheemmamoowala requested a review from mourner July 3, 2018 21:01

@mourner mourner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's just add the id all the time. We don't benefit much from having this as an option.

Comment thread index.js Outdated
});
tags: c.numPoints ? getClusterProperties(c) : this.points[c.index].properties
};
if (c.id) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

id can be 0 so let's do a !== undefined guard here.

Comment thread index.js Outdated
zoom: Infinity, // the last zoom the point was processed at
id: id, // index of the source feature in the original input array
index: id, // index of the source feature in the original input array,
id: p.id, // id of the source feature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't have to store original feature ids in the cluster objects because we can trace them back from original features (i.e. points[p.index].id).

@mourner mourner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, pending an id=0 fix.

Comment thread index.js Outdated
tags: c.numPoints ? getClusterProperties(c) : this.points[c.index].properties
};
const id = c.numPoints ? c.id : this.points[c.index].id;
if (id) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

id !== undefined to support id 0

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