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

Encode boolean values as uppercase string in WMS request. #1132

Merged
merged 2 commits into from
Feb 3, 2022

Conversation

stou
Copy link
Contributor

@stou stou commented Jan 17, 2022

Handle WMS server that only accepts boolean values as uppercase strings.

Handle WMS server that only accepts boolean values as uppercase strings.
Copy link
Contributor

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

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

Feels ok to me.

@JaffaKetchup
Copy link
Member

Forgive me if I'm wrong, but couldn't you just pass in the boolean string as uppercase anyway?

@ibrierley
Copy link
Contributor

I think currently it would have to be a bool rather than a string, and hence toString would be lowercase if I'm not mistaken (quite possible).

@@ -349,6 +349,9 @@ class WMSTileLayerOptions {
/// tile transparency flag
final bool transparent;
Copy link
Member

Choose a reason for hiding this comment

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

@ibrierley @stou Ah yes, I see now :)

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM. Just waiting on maintainers to return or people to be given write access. See #1117.

@JaffaKetchup
Copy link
Member

@stou Please pull upstream when possible, then I (or another maintainer) can merge.

@stou
Copy link
Contributor Author

stou commented Feb 3, 2022

@JaffaKetchup I have merged in the latest. Looks like the analysis and tests passes now :-)

@ibrierley ibrierley merged commit 85d979a into fleaflet:master Feb 3, 2022
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.

3 participants