-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add a visible() and selectionAPI() functions to layers. #682
Conversation
Note that, much like feature level visibility, you might need to call layer.draw() after making the layer visible to ensure an update. For feature layers, visibility cascades down to individual features (which disables their interaction).
Codecov Report
@@ Coverage Diff @@
## master #682 +/- ##
==========================================
+ Coverage 95.09% 95.14% +0.05%
==========================================
Files 83 83
Lines 8622 8678 +56
==========================================
+ Hits 8199 8257 +58
+ Misses 423 421 -2
Continue to review full report at Codecov.
|
The active property can now be changed.
* | ||
* @param {boolean|undefined} val: undefined to return the visibility, a | ||
* boolean to change the visibility. | ||
* @param {boolean} direct: if true, when getting the visibility, disregard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use of direct in two different ways is little confusing but I see a reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
direct
is only ever passed internally -- the user should never have to reference it. It is only so we know what the base visibility of a feature is regardless of the layer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right but I could see a use-case (perhaps more complicated) where a user would like to know the visible state of a feature not w.r.t to its parent. But I agree that most likely they would just query the state w.r.t parent.
@@ -339,13 +339,14 @@ var pixelmapFeature = function (arg) { | |||
m_quadFeature = m_this.layer().createFeature('quad', { | |||
selectionAPI: false, | |||
gcs: m_this.gcs(), | |||
visible: m_this.visible() | |||
visible: m_this.visible(undefined, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would have been nice to just call it like m_this.visible(true). Should we switch the args placeholder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but that means that the user would have to provide answer to direct
in their call, unless they are okay with passing undefined for direct in their code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user just uses m_this.visible(true)
. This is just done, because we don't want to change the state of visibility based on the level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Roger that, on the same page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks reasonable to me, just has some minor questions..
I am going back and forth on direct. Typically in the past for state change / query I used bitwise operators that could provide bit more flexibility and clarification but that means that would have remember the magic does For now like the names enough that I +2 it. |
In summary:
+2 |
The user really shouldn't ever call |
Note that, much like feature level visibility, you might need to call layer.draw() after making the layer visible to ensure an update. For feature layers, visibility cascades down to individual features (which disables their interaction).
This resolves issue #674.