-
Notifications
You must be signed in to change notification settings - Fork 742
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
Retrieve map style url from .well-known #5176
Conversation
Matrix SDKIntegration Tests Results:
|
val mapUrl = buildString { | ||
append(MAP_BASE_URL) | ||
append(keyParam) | ||
suspend fun getMapUrl(): String { |
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.
small nit: this might be easier to parse if we separate the fallback with its own variable
private val fallbackMapUrl = buildString {
append(MAP_BASE_URL)
append(keyParam)
}
suspend fun getMapUrl(): String {
val upstreamMapUrl = tryOrNull { rawService.getElementWellknown(session.sessionParams) }
?.mapTileServerConfig
?.mapStyleUrl
return upstreamMapUrl ?: fallbackMapUrl
}
what do you think?
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.
Much easier to read! I will commit, thanks.
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.
Done.
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.
LGTM 💯
?.mapStyleUrl | ||
return upstreamMapUrl ?: fallbackMapUrl | ||
} | ||
|
||
fun buildStaticMapUrl(locationData: LocationData, |
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.
I think this fun should also be impacted by the url set up in the .well-known file, since AFAIU, it can contain another API key.
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.
Nice catch.
Fixes #5175.
MSC