-
Notifications
You must be signed in to change notification settings - Fork 758
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 location crash #5084
Merged
+572
−316
Merged
Fix location crash #5084
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
7242f1c
Fix a crash when disabling the location on the device
bmarty db3f60b
Inform the user when the location provider is disabled.
bmarty e3242f0
Prevent the dialog from being cancellable, since OK button finish the…
bmarty e9b9406
Rework the location code - WIP
bmarty 1f53945
Rework the location code - WIP
bmarty 55ed737
Rework the location code - WIP
bmarty 26c0fee
Add a loader waiting for the user location to be known
bmarty 0f8c3bc
Try to get location by using all available providers.
onurays 4026ddb
Fix multiple pin rendering.
onurays 50279e3
Use static map image in timeline.
onurays 2dc52da
Use static map image in bottom sheet.
onurays e0ac8ee
No need for an extra FrameLayout
bmarty eff6942
Use a MaterialCarView
bmarty 2ce3894
Create a UrlMapProvider for a better handling of RTL languages, and b…
bmarty b14e557
Use the existing item click mechanism
bmarty 83ed80e
Rename fun for clarity
bmarty 303a858
Create an extension, improve the parsing algorithm, add robustness an…
bmarty 99f82d9
Avoid taking into account network location if we have gps location.
bmarty a8c251f
Avoid taking into account any provider location if we have gps location.
bmarty 2fbb434
Format
bmarty ecd41d3
network "not live" lcoation can be more accurate than GPS "not live" …
bmarty 4e3c730
Changelog
bmarty 8ee23c1
Merge branch 'develop' into feature/bma/location_crash
bmarty File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<resources> | ||
|
||
<bool name="is_rtl">true</bool> | ||
|
||
</resources> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ import im.vector.app.core.glide.GlideApp | |
import im.vector.app.core.utils.DimensionConverter | ||
import im.vector.app.features.home.AvatarRenderer | ||
import org.matrix.android.sdk.api.util.toMatrixItem | ||
import timber.log.Timber | ||
import javax.inject.Inject | ||
import javax.inject.Singleton | ||
|
||
|
@@ -54,22 +55,36 @@ class LocationPinProvider @Inject constructor( | |
val size = dimensionConverter.dpToPx(44) | ||
avatarRenderer.render(glideRequests, it, object : CustomTarget<Drawable>(size, size) { | ||
override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) { | ||
val bgUserPin = ContextCompat.getDrawable(context, R.drawable.bg_map_user_pin)!! | ||
val layerDrawable = LayerDrawable(arrayOf(bgUserPin, resource)) | ||
val horizontalInset = dimensionConverter.dpToPx(4) | ||
val topInset = dimensionConverter.dpToPx(4) | ||
val bottomInset = dimensionConverter.dpToPx(8) | ||
layerDrawable.setLayerInset(1, horizontalInset, topInset, horizontalInset, bottomInset) | ||
|
||
cache[userId] = layerDrawable | ||
|
||
callback(layerDrawable) | ||
Timber.d("## Location: onResourceReady") | ||
val pinDrawable = createPinDrawable(resource) | ||
cache[userId] = pinDrawable | ||
callback(pinDrawable) | ||
} | ||
|
||
override fun onLoadCleared(placeholder: Drawable?) { | ||
// Is it possible? Put placeholder instead? | ||
// FIXME The doc says it has to be implemented and should free resources | ||
Timber.d("## Location: onLoadCleared") | ||
} | ||
|
||
override fun onLoadFailed(errorDrawable: Drawable?) { | ||
Timber.w("## Location: onLoadFailed") | ||
errorDrawable ?: return | ||
val pinDrawable = createPinDrawable(errorDrawable) | ||
cache[userId] = pinDrawable | ||
callback(pinDrawable) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes the issue when the user does not have an avatar |
||
} | ||
}) | ||
} | ||
} | ||
|
||
private fun createPinDrawable(drawable: Drawable): Drawable { | ||
val bgUserPin = ContextCompat.getDrawable(context, R.drawable.bg_map_user_pin)!! | ||
val layerDrawable = LayerDrawable(arrayOf(bgUserPin, drawable)) | ||
val horizontalInset = dimensionConverter.dpToPx(4) | ||
val topInset = dimensionConverter.dpToPx(4) | ||
val bottomInset = dimensionConverter.dpToPx(8) | ||
layerDrawable.setLayerInset(1, horizontalInset, topInset, horizontalInset, bottomInset) | ||
return layerDrawable | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
nit: is it worth extracting these to width/height constants or using named params?
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, and I think the values are not correct (since not related to the actual ImageView size). Anyway, could be improved later, this is not a big issue, Glide will crop the image anyway (I think)
Thanks for the review!