-
-
Notifications
You must be signed in to change notification settings - Fork 20
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 popup actions #208
fix popup actions #208
Conversation
@@ -727,9 +731,9 @@ export function buildJsQuery(dotNetQuery: DotNetQuery): Query { | |||
where: dotNetQuery.where ?? "1=1", | |||
spatialRelationship: dotNetQuery.spatialRelationship as any ?? "intersects", | |||
distance: dotNetQuery.distance ?? undefined, | |||
units: dotNetQuery.units as any ?? null, |
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.
https://developers.arcgis.com/javascript/latest/api-reference/esri-rest-support-Query.html#units
The default value is null per the API?
returnGeometry: dotNetQuery.returnGeometry ?? false, | ||
outFields: dotNetQuery.outFields ?? null, |
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.
Same here, default value is null:
https://developers.arcgis.com/javascript/latest/api-reference/esri-rest-support-Query.html#outFields
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.
Yes, I haven't actually improved anything here, but I also didn't hurt anything. I think I made this change as part of an investigation into an issue, and it seemed to make more sense to me to keep it undefined
as more consistent.
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.
Just a couple comments on the default values that got updated.
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.
Looks good. I had one question about possibly adding a note in the reactiveUtils
or popup
documentation about this method
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.
TypeScript looks good to me :)
|
Closes #207
The
view.popup.on
function is not available until the view is rendered. Rather than do some sort of re-registration of thepopupTemplate
, I realized we could use areactiveUtils.once
to register the handler after the popup is ready.Also in this PR
undefined
s added to ajsBuilder
to prevent null errorscopyAssets.ps1
. Powershell's default behavior seems to have changed. Christopher and I saw this before, now my version matches what his Mac was doing 3 months ago...