Feature request for https://github.com/CesiumGS/cesium/issues/9545

[martin-bom]

Fixes #9545

[cesium-concierge]

Thank you so much for the pull request @martin-bom! I noticed this is your first pull request and I wanted to say welcome to the Cesium community!

The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.

• ❌ Missing CONTRIBUTORS.md entry.
  • Please add yourself to the contributors file!
• ❌ Missing CLA.
  • Please send in a Contributor License Agreement (CLA) and comment back here to let us know to check this!
  • Maintainers, here's how to check for a submitted CLA.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[ebogo1]

Hi @martin-bom - thanks for opening the PR! Could you please add a test in WebMapServiceImageryProviderSpec to make sure this works as expected?

[martin-bom]

Hi @martin-bom - thanks for opening the PR! Could you please add a test in WebMapServiceImageryProviderSpec to make sure this works as expected?

Hi @ebogo1 I will add it and come back to you. Thank you.

[ebogo1]

@martin-bom Apologies for the delay, some more feedback:

• There should be an entry in the WebMapServiceImageryProvider constructor for the new getFeatureInfoUrl option.
• CHANGES.md should be updated with a note that this new option was added to WebMapServiceImageryProvider.
• Please add your name to CONTRIBUTORS.md :) (see the concierge's comment above for details). And I can see you've since submitted the CLA, thanks!

[cesium-concierge]

Thanks again for your contribution @martin-bom!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[martin-bom]

@ebogo1 Hello, I have made the changes based on your last review. Can you help to review again? Thank you.

[ebogo1]

Thanks @martin-bom - the new changes look good to me. I wonder if getFeatureInfoUrl is a better name than featureInfoUrl, but otherwise I think this is good to go.

@tfili - could you please take another look when you get a chance? Thanks!

[martin-bom]

Thanks @martin-bom - the new changes look good to me. I wonder if getFeatureInfoUrl is a better name than featureInfoUrl, but otherwise I think this is good to go.

@tfili - could you please take another look when you get a chance? Thanks!

Hi @ebogo1 thank you for the review. I took away the "get" because when I came to the get/set definition and thought getGetFeatureInfoUrl was a bit awkward. I am more than happy to rename it back to getFeatureInfoUrl as getFeatureInfo is a service request name. Thank you.

[lilleyse]

Thanks for the updates here @martin-bom. I pushed some small changes to help move along the PR.

I removed the getFeatureInfoUrl setter since it wasn't propagating fully to the UrlTemplateImageryProvider. The function is marked as @readonly anyways so I assume the setter wasn't really needed in the first place. Let me know if it should be added and we can think about doing a follow up PR.