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

Location centring user's sharing location #7398

Merged
merged 8 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion Riot/Modules/MatrixKit/Models/Room/MXKRoomDataSourceManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,21 @@ @interface MXKRoomDataSourceManager()
*/
NSMutableDictionary *roomDataSources;

/**
The list of rooms with a late event decrypt. Causing bubbles issues
Each element is a room ID.
*/
NSMutableSet *roomDataSourcesToDestroy;

/**
Observe UIApplicationDidReceiveMemoryWarningNotification to dispose of any resources that can be recreated.
*/
id UIApplicationDidReceiveMemoryWarningNotificationObserver;

/**
Observe kMXEventDidDecryptNotification to get late decrypted events.
*/
id mxEventDidDecryptNotificationObserver;
}

@end
Expand Down Expand Up @@ -119,6 +130,7 @@ - (instancetype)initWithMatrixSession:(MXSession *)matrixSession
{
mxSession = matrixSession;
roomDataSources = [NSMutableDictionary dictionary];
roomDataSourcesToDestroy = [NSMutableSet set];
_releasePolicy = MXKRoomDataSourceManagerReleasePolicyNeverRelease;

[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(didMXSessionDidLeaveRoom:) name:kMXSessionDidLeaveRoomNotification object:nil];
Expand All @@ -138,6 +150,12 @@ - (instancetype)initWithMatrixSession:(MXSession *)matrixSession
}

}];

// Observe late decrypted events, and store rooms ids in memory
mxEventDidDecryptNotificationObserver = [[NSNotificationCenter defaultCenter] addObserverForName:kMXEventDidDecryptNotification object:nil queue:[NSOperationQueue mainQueue] usingBlock:^(NSNotification *notif) {
MXEvent *decryptedEvent = notif.object;
[self->roomDataSourcesToDestroy addObject:decryptedEvent.roomId];
}];
}
return self;
}
Expand All @@ -156,6 +174,11 @@ - (void)destroy
[[NSNotificationCenter defaultCenter] removeObserver:UIApplicationDidReceiveMemoryWarningNotificationObserver];
UIApplicationDidReceiveMemoryWarningNotificationObserver = nil;
}
if (mxEventDidDecryptNotificationObserver)
{
[[NSNotificationCenter defaultCenter] removeObserver:mxEventDidDecryptNotificationObserver];
mxEventDidDecryptNotificationObserver = nil;
}
}

#pragma mark
Expand Down Expand Up @@ -202,9 +225,19 @@ - (void)roomDataSourceForRoom:(NSString *)roomId create:(BOOL)create onComplete:

// If not available yet, create the room data source
MXKRoomDataSource *roomDataSource = roomDataSources[roomId];


// check if the room's dataSource has events with late decryption issues and destroys it
BOOL roomDataSourceToBeDestroyed = [roomDataSourcesToDestroy containsObject:roomId];

if (roomDataSource && roomDataSourceToBeDestroyed && create) {
[roomDataSource destroy];
roomDataSources[roomId] = nil;
roomDataSource = nil;
}

aringenbach marked this conversation as resolved.
Show resolved Hide resolved
if (!roomDataSource && create && roomId)
{
[roomDataSourcesToDestroy removeObject:roomId];
[_roomDataSourceClass loadRoomDataSourceWithRoomId:roomId threadId:nil andMatrixSession:mxSession onComplete:^(id roomDataSource) {
[self addRoomDataSource:roomDataSource];
onComplete(roomDataSource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct LiveLocationSharingViewerViewState: BindableState {
/// Live location list items
var listItemsViewData: [LiveLocationListItemViewData]

var showsUserLocation = false
var showsUserLocationMode: ShowUserLocationMode = .hide

var isCurrentUserShared: Bool {
listItemsViewData.contains { $0.isCurrentUser }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ class LiveLocationSharingViewerViewModel: LiveLocationSharingViewerViewModelType
guard let foundUserAnnotation = foundUserAnnotation else {
return
}

if state.showsUserLocationMode == .follow {
state.showsUserLocationMode = .show
}
flescio marked this conversation as resolved.
Show resolved Hide resolved
state.highlightedAnnotation = foundUserAnnotation
}

Expand All @@ -234,7 +236,7 @@ class LiveLocationSharingViewerViewModel: LiveLocationSharingViewerViewModelType

private func showsCurrentUserLocation() {
if liveLocationSharingViewerService.requestAuthorizationIfNeeded() {
state.showsUserLocation = true
state.showsUserLocationMode = .follow
} else {
state.errorSubject.send(.invalidLocationAuthorization)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ class LiveLocationSharingViewerViewModelTests: XCTestCase {
let service = MockLiveLocationSharingViewerService(currentUserSharingLocation: false)
let viewModel = LiveLocationSharingViewerViewModel(mapStyleURL: BuildSettings.defaultTileServerMapStyleURL, service: service)
XCTAssertFalse(viewModel.context.viewState.isCurrentUserShared)
XCTAssertFalse(viewModel.context.viewState.showsUserLocation)
XCTAssertEqual(viewModel.context.viewState.showsUserLocationMode, .hide)
viewModel.context.send(viewAction: .showUserLocation)
XCTAssertTrue(viewModel.context.viewState.showsUserLocation)
XCTAssertEqual(viewModel.context.viewState.showsUserLocationMode, .follow)
viewModel.context.send(viewAction: .tapListItem("@bob:matrix.org"))
XCTAssertEqual(viewModel.context.viewState.showsUserLocationMode, .show)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct LiveLocationSharingViewer: View {
annotations: viewModel.viewState.annotations,
highlightedAnnotation: viewModel.viewState.highlightedAnnotation,
userAvatarData: nil,
showsUserLocation: viewModel.viewState.showsUserLocation,
showsUserLocationMode: viewModel.viewState.showsUserLocationMode,
userAnnotationCanShowCallout: true,
userLocation: Binding.constant(nil),
mapCenterCoordinate: Binding.constant(nil),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ import Combine
import Mapbox
import SwiftUI

enum ShowUserLocationMode {
case follow
case show
case hide
}
flescio marked this conversation as resolved.
Show resolved Hide resolved

struct LocationSharingMapView: UIViewRepresentable {
// MARK: - Constants

Expand All @@ -40,7 +46,7 @@ struct LocationSharingMapView: UIViewRepresentable {
let userAvatarData: AvatarInputProtocol?

/// True to indicate to show and follow current user location
var showsUserLocation = false
var showsUserLocationMode: ShowUserLocationMode = .hide
flescio marked this conversation as resolved.
Show resolved Hide resolved

/// True to indicate that a touch on user annotation can show a callout
var userAnnotationCanShowCallout = false
Expand Down Expand Up @@ -75,14 +81,18 @@ struct LocationSharingMapView: UIViewRepresentable {
mapView.vc_removeAllAnnotations()
mapView.addAnnotations(annotations)

if let highlightedAnnotation = highlightedAnnotation, !showsUserLocation {
mapView.setCenter(highlightedAnnotation.coordinate, zoomLevel: Constants.mapZoomLevel, animated: false)
if let highlightedAnnotation = highlightedAnnotation, showsUserLocationMode != .follow {
alfogrillo marked this conversation as resolved.
Show resolved Hide resolved
mapView.setCenter(highlightedAnnotation.coordinate, zoomLevel: Constants.mapZoomLevel, animated: true)
}

if showsUserLocation {
switch showsUserLocationMode {
case .follow:
mapView.showsUserLocation = true
mapView.userTrackingMode = .follow
} else {
case .show:
mapView.showsUserLocation = true
mapView.userTrackingMode = .none
case .hide:
mapView.showsUserLocation = false
mapView.userTrackingMode = .none
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct LocationSharingViewState: BindableState {
var showLoadingIndicator = false

/// True to indicate to show and follow current user location
var showsUserLocation = false
var showsUserLocationMode: ShowUserLocationMode = .hide
flescio marked this conversation as resolved.
Show resolved Hide resolved

/// Used to hide live location sharing features
var isLiveLocationSharingEnabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class LocationSharingViewModel: LocationSharingViewModelType, LocationSharingVie
userAvatarData: avatarData,
annotations: [],
highlightedAnnotation: nil,
showsUserLocation: true,
showsUserLocationMode: .follow,
isLiveLocationSharingEnabled: isLiveLocationSharingEnabled)

super.init(initialViewState: viewState)
Expand Down Expand Up @@ -73,15 +73,15 @@ class LocationSharingViewModel: LocationSharingViewModelType, LocationSharingVie

completion?(.share(latitude: pinLocation.latitude, longitude: pinLocation.longitude, coordinateType: .pin))
case .goToUserLocation:
state.showsUserLocation = true
state.showsUserLocationMode = .follow
state.isPinDropSharing = false
case .startLiveSharing:
startLiveLocationSharing()
case .shareLiveLocation(let timeout):
state.bindings.showingTimerSelector = false
completion?(.shareLiveLocation(timeout: timeout.rawValue))
case .userDidPan:
state.showsUserLocation = false
state.showsUserLocationMode = .hide
state.isPinDropSharing = true
case .mapCreditsDidTap:
state.bindings.showMapCreditsSheet.toggle()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ struct LocationSharingView: View {
annotations: context.viewState.annotations,
highlightedAnnotation: context.viewState.highlightedAnnotation,
userAvatarData: context.viewState.userAvatarData,
showsUserLocation: context.viewState.showsUserLocation,
showsUserLocationMode: context.viewState.showsUserLocationMode,
userLocation: $context.userLocation,
mapCenterCoordinate: $context.pinLocation,
errorSubject: context.viewState.errorSubject,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ struct StaticLocationViewingViewState: BindableState {
/// Shared annotation to display existing location
let sharedAnnotation: LocationAnnotation

var showsUserLocation = false
var showsUserLocationMode: ShowUserLocationMode = .hide

var showLoadingIndicator = false

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class StaticLocationViewingViewModel: StaticLocationViewingViewModelType, Static

private func showsCurrentUserLocation() {
if staticLocationSharingViewerService.requestAuthorizationIfNeeded() {
state.showsUserLocation = true
state.showsUserLocationMode = .follow
} else {
state.errorSubject.send(.invalidLocationAuthorization)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ class StaticLocationViewingViewModelTests: XCTestCase {

func testToggleShowUserLocation() {
let viewModel = buildViewModel()
XCTAssertFalse(viewModel.context.viewState.showsUserLocation)
XCTAssertEqual(viewModel.context.viewState.showsUserLocationMode, .hide)
viewModel.context.send(viewAction: .showUserLocation)
XCTAssertTrue(viewModel.context.viewState.showsUserLocation)
XCTAssertEqual(viewModel.context.viewState.showsUserLocationMode, .follow)
alfogrillo marked this conversation as resolved.
Show resolved Hide resolved
}

private func buildViewModel() -> StaticLocationViewingViewModel {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct StaticLocationView: View {
annotations: [viewModel.viewState.sharedAnnotation],
highlightedAnnotation: viewModel.viewState.sharedAnnotation,
userAvatarData: nil,
showsUserLocation: viewModel.viewState.showsUserLocation,
showsUserLocationMode: viewModel.viewState.showsUserLocationMode,
userLocation: Binding.constant(nil),
mapCenterCoordinate: Binding.constant(nil),
errorSubject: viewModel.viewState.errorSubject)
Expand Down
1 change: 1 addition & 0 deletions changelog.d/7397.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix issue on timeline's bubbles not showing proper content after decrypt
1 change: 1 addition & 0 deletions changelog.d/pr-7398.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes bug about centring user in live location sharing