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

Fix a bug with polygon stroke styles. #700

Merged
merged 1 commit into from
Jun 2, 2017
Merged

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Jun 1, 2017

If a polygon has a hole, then stroke styles were misapplied to polygons. This was caused by there being more than one stroke per polygon and the stroke styles being called with polygon indices.

Fixes #699.

If a polygon has a hole, then stroke styles were misapplied to polygons.  This was caused by there being more than one stroke per polygon and the stroke styles being called with polygon indices.

Fixes #699.
@codecov-io
Copy link

Codecov Report

Merging #700 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #700      +/-   ##
==========================================
+ Coverage   95.25%   95.28%   +0.02%     
==========================================
  Files          83       83              
  Lines        8985     8990       +5     
==========================================
+ Hits         8559     8566       +7     
+ Misses        426      424       -2
Impacted Files Coverage Δ
src/polygonFeature.js 99.34% <100%> (+0.02%) ⬆️
src/mapInteractor.js 96.12% <0%> (+0.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 248adae...85b595d. Read the comment docs.

Copy link
Member

@aashish24 aashish24 left a comment

Choose a reason for hiding this comment

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

Just has one minor comment but other than that it looks good to me. It would be good if matthew or @jbeezley can confirm it fixes the issue.

*
* @param {(object|function)?} styleValue The polygon's style value used for
* the stroke. This should be m_this.style(<name of style>) and not
* m_this.style.get(<name of style>), as the result is more efficient if
Copy link
Member

Choose a reason for hiding this comment

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

I have a feeling that this comment can be made more readable. Also when we refer to get, are we talking about Map.prototype.get()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our style function can either be called like style('strokeColor'), which returns the value set for strokeColor, or as style.get('strokeColor'), which always returns a function for strokeColor. Here, if we know that the value is not a function, then we can pass it along to the stroke, whereas if it is a function, we have to get the appropriate value for the polygon by calling that function with the polygon's values.

Copy link
Contributor

@jbeezley jbeezley left a comment

Choose a reason for hiding this comment

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

I can confirm that it fixes the issue. Thanks @manthey.

@manthey manthey merged commit 31bf6e5 into master Jun 2, 2017
@manthey manthey deleted the polygon-stroke-with-holes branch June 2, 2017 12:45
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.

4 participants