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 dynamic layer identifies for sublayer id 0 #706

Merged
merged 7 commits into from
Apr 12, 2017
Merged

Fix dynamic layer identifies for sublayer id 0 #706

merged 7 commits into from
Apr 12, 2017

Conversation

green3g
Copy link
Member

@green3g green3g commented Mar 29, 2017

Description

Fix bug #675 with better check for valid layer id

Checklist

  • grunt lint produces no error messages

@@ -34,6 +34,21 @@ define([
// for details on pop-up definition see: https://developers.arcgis.com/javascript/jshelp/intro_popuptemplate.html

identifies: {
population: {
Copy link
Member

Choose a reason for hiding this comment

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

Looking towards the future, I would like to avoid using services from sampleserver1 (and 2, 3, and 4). These older servers at version 10.0 are problematic for use with version 4.x (specifically when using 3D maps). This applies to the existing Louisville Public Safety service and SF 311 services as well. Public Safety Service is where the problems with 4.x surfaced.

For now, how about we remove or replace the Population service in this PR? (replace from sampleserver6?) Then we can replace the other out-of-date services in a separate PR. Thoughts?

visible: true,
fieldName: 'POP',
label: 'Population',
formatter: commafy
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't dojo's number.format be a better practice to promote - automatically handling differences based on locale?

@tmcgee
Copy link
Member

tmcgee commented Apr 10, 2017

@roemhildtg I have tested the change to Identify and verified this works. I made a few additional changes. Please review and comment. thanks.

@green3g
Copy link
Member Author

green3g commented Apr 11, 2017

Yes, this looks good to me.

@tmcgee tmcgee merged commit d46346b into develop Apr 12, 2017
@green3g green3g deleted the fix-675 branch April 20, 2017 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants