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

enable geolocation behaviour in location picker for live share type #8068

Merged
merged 2 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 1 addition & 3 deletions res/css/components/views/location/_ShareType.scss
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ limitations under the License.
border-radius: 50%;

&.Own {
// color is set by user color class
// generated from id
border-color: currentColor;
border-color: $accent;
}

&.Live {
Expand Down
12 changes: 10 additions & 2 deletions res/css/views/location/_LocationPicker.scss
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ limitations under the License.
width: 31px;
height: 31px;
border-radius: 50%;
background-color: $accent;
filter: drop-shadow(0px 3px 5px rgba(0, 0, 0, 0.2));
background-color: currentColor;

display: flex;
align-items: center;
Expand All @@ -87,7 +87,7 @@ limitations under the License.
width: 9px;
height: 5px;
position: absolute;
background-color: $accent;
background-color: currentColor;
}
}
}
Expand Down Expand Up @@ -135,3 +135,11 @@ limitations under the License.
width: 100%;
height: 48px;
}

// live marker color is set by user color class
// generated from userid
// others are $accent
.mx_MLocationBody_marker-Self,
.mx_MLocationBody_marker-Pin {
color: $accent;
}
26 changes: 15 additions & 11 deletions src/components/views/location/LocationPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import maplibregl, { MapMouseEvent } from 'maplibre-gl';
import { logger } from "matrix-js-sdk/src/logger";
import { RoomMember } from 'matrix-js-sdk/src/models/room-member';
import { ClientEvent, IClientWellKnown } from 'matrix-js-sdk/src/client';
import classNames from 'classnames';

import { _t } from '../../../languageHandler';
import { replaceableComponent } from "../../../utils/replaceableComponent";
Expand All @@ -33,6 +34,7 @@ import { Icon as LocationIcon } from '../../../../res/img/element-icons/location
import { LocationShareError } from './LocationShareErrors';
import AccessibleButton from '../elements/AccessibleButton';
import { MapError } from './MapError';
import { getUserNameColorClass } from '../../../utils/FormattingUtils';
export interface ILocationPickerProps {
sender: RoomMember;
shareType: LocationShareType;
Expand All @@ -52,13 +54,8 @@ interface IState {
error?: LocationShareError;
}

/*
* An older version of this file allowed manually picking a location on
* the map to share, instead of sharing your current location.
* Since the current designs do not cover this case, it was removed from
* the code but you should be able to find it in the git history by
* searching for the commit that remove manualPosition from this file.
*/
const isSharingOwnLocation = (shareType: LocationShareType): boolean =>
shareType === LocationShareType.Own || shareType === LocationShareType.Live;

@replaceableComponent("views.location.LocationPicker")
class LocationPicker extends React.Component<ILocationPickerProps, IState> {
Expand Down Expand Up @@ -117,7 +114,7 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> {

this.geolocate.on('error', this.onGeolocateError);

if (this.props.shareType === LocationShareType.Own) {
if (isSharingOwnLocation(this.props.shareType)) {
this.geolocate.on('geolocate', this.onGeolocate);
}

Expand Down Expand Up @@ -191,7 +188,7 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> {
logger.error("Could not fetch location", e);
// close the dialog and show an error when trying to share own location
// pin drop location without permissions is ok
if (this.props.shareType === LocationShareType.Own) {
if (isSharingOwnLocation(this.props.shareType)) {
this.props.onFinished();
Modal.createTrackedDialog(
'Could not fetch location',
Expand Down Expand Up @@ -225,6 +222,8 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> {
</div>;
}

const userColorClass = getUserNameColorClass(this.props.sender.userId);

return (
<div className="mx_LocationPicker">
<div id="mx_LocationPicker_map" />
Expand All @@ -249,9 +248,14 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> {
</AccessibleButton>
</form>
</div>
<div className="mx_MLocationBody_marker" id={this.getMarkerId()}>
<div className={classNames(
"mx_MLocationBody_marker",
`mx_MLocationBody_marker-${this.props.shareType}`,
userColorClass,
)}
id={this.getMarkerId()}>
<div className="mx_MLocationBody_markerBorder">
{ this.props.shareType === LocationShareType.Own ?
{ isSharingOwnLocation(this.props.shareType) ?
<MemberAvatar
member={this.props.sender}
width={27}
Expand Down
4 changes: 1 addition & 3 deletions src/components/views/location/ShareType.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import React, { HTMLAttributes, useContext } from 'react';
import MatrixClientContext from '../../../contexts/MatrixClientContext';
import { _t } from '../../../languageHandler';
import { OwnProfileStore } from '../../../stores/OwnProfileStore';
import { getUserNameColorClass } from '../../../utils/FormattingUtils';
import BaseAvatar from '../avatars/BaseAvatar';
import AccessibleButton from '../elements/AccessibleButton';
import Heading from '../typography/Heading';
Expand All @@ -34,9 +33,8 @@ const UserAvatar = () => {
// 40 - 2px border
const avatarSize = 36;
const avatarUrl = OwnProfileStore.instance.getHttpAvatarUrl(avatarSize);
const colorClass = getUserNameColorClass(userId);

return <div className={`mx_ShareType_option-icon ${LocationShareType.Own} ${colorClass}`}>
return <div className={`mx_ShareType_option-icon ${LocationShareType.Own}`}>
<BaseAvatar
idName={userId}
name={displayName}
Expand Down
103 changes: 54 additions & 49 deletions test/components/views/location/LocationPicker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,61 +205,66 @@ describe("LocationPicker", () => {
expect(mockGeolocate.trigger).toHaveBeenCalled();
});

describe('for Own location share type', () => {
it('closes and displays error when geolocation errors', () => {
// suppress expected error log
jest.spyOn(logger, 'error').mockImplementation(() => { });
const onFinished = jest.fn();
getComponent({ onFinished });

expect(mockMap.addControl).toHaveBeenCalledWith(mockGeolocate);
act(() => {
// @ts-ignore
mockMap.emit('load');
// @ts-ignore
mockGeolocate.emit('error', {});
const testUserLocationShareTypes = (shareType: LocationShareType.Own | LocationShareType.Live) => {
describe(`for ${shareType} location share type`, () => {
it('closes and displays error when geolocation errors', () => {
// suppress expected error log
jest.spyOn(logger, 'error').mockImplementation(() => { });
const onFinished = jest.fn();
getComponent({ onFinished, shareType });

expect(mockMap.addControl).toHaveBeenCalledWith(mockGeolocate);
act(() => {
// @ts-ignore
mockMap.emit('load');
// @ts-ignore
mockGeolocate.emit('error', {});
});

// dialog is closed on error
expect(onFinished).toHaveBeenCalled();
});

// dialog is closed on error
expect(onFinished).toHaveBeenCalled();
});

it('sets position on geolocate event', () => {
const wrapper = getComponent();
act(() => {
// @ts-ignore
mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition);
wrapper.setProps({});
it('sets position on geolocate event', () => {
const wrapper = getComponent({ shareType });
act(() => {
// @ts-ignore
mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition);
wrapper.setProps({});
});

// marker added
expect(maplibregl.Marker).toHaveBeenCalled();
expect(mockMarker.setLngLat).toHaveBeenCalledWith(new maplibregl.LngLat(
12.4, 43.2,
));
// submit button is enabled when position is truthy
expect(findByTestId(wrapper, 'location-picker-submit-button').at(0).props().disabled).toBeFalsy();
expect(wrapper.find('MemberAvatar').length).toBeTruthy();
});

// marker added
expect(maplibregl.Marker).toHaveBeenCalled();
expect(mockMarker.setLngLat).toHaveBeenCalledWith(new maplibregl.LngLat(
12.4, 43.2,
));
// submit button is enabled when position is truthy
expect(findByTestId(wrapper, 'location-picker-submit-button').at(0).props().disabled).toBeFalsy();
expect(wrapper.find('MemberAvatar').length).toBeTruthy();
});

it('submits location', () => {
const onChoose = jest.fn();
const wrapper = getComponent({ onChoose });
act(() => {
// @ts-ignore
mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition);
// make sure button is enabled
wrapper.setProps({});
it('submits location', () => {
const onChoose = jest.fn();
const wrapper = getComponent({ onChoose, shareType });
act(() => {
// @ts-ignore
mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition);
// make sure button is enabled
wrapper.setProps({});
});

act(() => {
findByTestId(wrapper, 'location-picker-submit-button').at(0).simulate('click');
});

// content of this call is tested in LocationShareMenu-test
expect(onChoose).toHaveBeenCalled();
});

act(() => {
findByTestId(wrapper, 'location-picker-submit-button').at(0).simulate('click');
});

// content of this call is tested in LocationShareMenu-test
expect(onChoose).toHaveBeenCalled();
});
});
};

testUserLocationShareTypes(LocationShareType.Own);
testUserLocationShareTypes(LocationShareType.Live);

describe('for Pin drop location share type', () => {
const shareType = LocationShareType.Pin;
Expand Down