Skip to content

Commit

Permalink
do not zoom if marker was dragged (graphhopper#193)
Browse files Browse the repository at this point in the history
* do not zoom if marker was dragged

* formatting
  • Loading branch information
karussell authored Jun 7, 2022
1 parent e775843 commit 006ed46
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 42 deletions.
4 changes: 3 additions & 1 deletion src/actions/Actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ export class InfoReceived implements Action {

export class SetPoint implements Action {
readonly point: QueryPoint
readonly zoom: boolean

constructor(point: QueryPoint) {
constructor(point: QueryPoint, zoom: boolean) {
this.point = point
this.zoom = zoom
}
}

Expand Down
1 change: 1 addition & 0 deletions src/api/graphhopper.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface RoutingArgs {
readonly points: [number, number][]
readonly profile: string
readonly maxAlternativeRoutes: number
readonly zoom: boolean
}

export interface RoutingRequest {
Expand Down
13 changes: 8 additions & 5 deletions src/layers/UseQueryPointsLayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,14 @@ function addDragInteractions(map: Map, queryPointsLayer: VectorLayer<any>) {
const coordinateLonLat = toLonLat(feature.getGeometry().getCoordinates())
const coordinate = { lng: coordinateLonLat[0], lat: coordinateLonLat[1] }
Dispatcher.dispatch(
new SetPoint({
...point,
coordinate,
queryText: coordinateToText(coordinate),
})
new SetPoint(
{
...point,
coordinate,
queryText: coordinateToText(coordinate),
},
false
)
)
})
modify.set('gh:drag_query_point', true)
Expand Down
15 changes: 9 additions & 6 deletions src/map/Popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ export function PopupComponent({
const dispatchSetPoint = function (point: QueryPoint, coordinate: Coordinate) {
onSelect()
Dispatcher.dispatch(
new SetPoint({
...point,
coordinate: coordinate,
queryText: coordinateToText(coordinate),
isInitialized: true,
})
new SetPoint(
{
...point,
coordinate: coordinate,
queryText: coordinateToText(coordinate),
isInitialized: true,
},
true
)
)
}

Expand Down
28 changes: 17 additions & 11 deletions src/sidebar/search/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,23 @@ const SearchBox = ({
onAddressSelected={(queryText, coordinate) =>
Dispatcher.dispatch(
coordinate
? new SetPoint({
...point,
isInitialized: true,
queryText: queryText,
coordinate: coordinate,
})
: new SetPoint({
...point,
isInitialized: false,
queryText: queryText,
})
? new SetPoint(
{
...point,
isInitialized: true,
queryText: queryText,
coordinate: coordinate,
},
true
)
: new SetPoint(
{
...point,
isInitialized: false,
queryText: queryText,
},
true
)
)
}
onChange={onChange}
Expand Down
2 changes: 1 addition & 1 deletion src/stores/MapActionReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default class MapActionReceiver implements ActionReceiver {
// this assumes that always the first path is selected as result. One could use the
// state of the routeStore as well but then we would have to make sure that the route
// store digests this action first, which our Dispatcher can't at the moment.
fitBounds(this.map, action.result.paths[0].bbox!, isSmallScreen)
if (action.request.zoom) fitBounds(this.map, action.result.paths[0].bbox!, isSmallScreen)
} else if (action instanceof SetSelectedPath) {
fitBounds(this.map, action.path.bbox!, isSmallScreen)
} else if (action instanceof PathDetailsRangeSelected) {
Expand Down
4 changes: 4 additions & 0 deletions src/stores/QueryStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface QueryStoreState {
readonly currentRequest: CurrentRequest
readonly maxAlternativeRoutes: number
readonly routingProfile: RoutingProfile
readonly zoom: boolean
}

export interface QueryPoint {
Expand Down Expand Up @@ -81,6 +82,7 @@ export default class QueryStore extends Store<QueryStoreState> {
routingProfile: {
name: '',
},
zoom: true,
}
}

Expand Down Expand Up @@ -112,6 +114,7 @@ export default class QueryStore extends Store<QueryStoreState> {
const newState: QueryStoreState = {
...state,
queryPoints: QueryStore.replacePoint(state.queryPoints, action.point),
zoom: action.zoom,
}

return this.routeIfAllPointsSet(newState)
Expand Down Expand Up @@ -297,6 +300,7 @@ export default class QueryStore extends Store<QueryStoreState> {
points: coordinates,
profile: state.routingProfile.name,
maxAlternativeRoutes: state.maxAlternativeRoutes,
zoom: state.zoom,
}
}

Expand Down
13 changes: 8 additions & 5 deletions test/NavBar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('NavBar', function () {

// modify state of stores which the nav bar depends on
for (const point of points) {
queryStore.receive(new SetPoint(point))
queryStore.receive(new SetPoint(point, true))
}
queryStore.receive(new SetVehicleProfile(profile))
mapStore.receive(new SelectMapStyle(style))
Expand Down Expand Up @@ -190,10 +190,13 @@ describe('NavBar', function () {
href: 'https://origin.com',
}
Dispatcher.dispatch(
new SetPoint({
...queryStore.state.queryPoints[0],
isInitialized: true,
})
new SetPoint(
{
...queryStore.state.queryPoints[0],
isInitialized: true,
},
true
)
)

//act
Expand Down
7 changes: 7 additions & 0 deletions test/routing/Api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe('route', () => {
points: [],
maxAlternativeRoutes: 1,
profile: 'profile',
zoom: true,
}
const mockedDispatcher = jest.spyOn(Dispatcher, 'dispatch')
const ghApi = 'https://some.api/'
Expand All @@ -105,6 +106,7 @@ describe('route', () => {
points: [],
maxAlternativeRoutes: 1,
profile: 'car',
zoom: true,
}

const expectedBody: RoutingRequest = {
Expand Down Expand Up @@ -138,6 +140,7 @@ describe('route', () => {
points: [],
maxAlternativeRoutes: 2,
profile: 'car',
zoom: true,
}

const expectedBody: RoutingRequest = {
Expand Down Expand Up @@ -177,6 +180,7 @@ describe('route', () => {
],
maxAlternativeRoutes: 1,
profile: 'bla',
zoom: true,
}

fetchMock.mockResponseOnce(JSON.stringify(getEmptyResult()))
Expand All @@ -197,6 +201,7 @@ describe('route', () => {
],
maxAlternativeRoutes: 1,
profile: 'bla',
zoom: true,
}

const error: ErrorResponse = {
Expand All @@ -219,6 +224,7 @@ describe('route', () => {
profile: 'car',
points: [],
maxAlternativeRoutes: 3,
zoom: true,
}
fetchMock.mockResponse(() => Promise.resolve({ status: 500 }))
await expect(new ApiImpl('https://some.api/', 'key').route(args)).rejects.toThrow('Route calculation timed out')
Expand All @@ -231,6 +237,7 @@ describe('route', () => {
points: [],
maxAlternativeRoutes: 1,
profile: 'car',
zoom: true,
}

const expectedBody: RoutingRequest = {
Expand Down
32 changes: 20 additions & 12 deletions test/stores/QueryStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('QueryStore', () => {
routingProfile: { name: 'car' },
}

const state = store.reduce(storeState, new SetPoint(point))
const state = store.reduce(storeState, new SetPoint(point, true))

expect(state.queryPoints[0]).toEqual(point)
})
Expand All @@ -69,7 +69,7 @@ describe('QueryStore', () => {
}

for (const point of store.state.queryPoints) {
state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }))
state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }, true))
}

// the store should not send anything unless points and routing profile are specified
Expand All @@ -88,7 +88,7 @@ describe('QueryStore', () => {
routingProfile: { name: 'car' },
}
for (const point of store.state.queryPoints) {
state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }))
state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }, true))
}

expect(state.queryPoints.every(point => point.isInitialized)).toBeTruthy()
Expand All @@ -106,7 +106,7 @@ describe('QueryStore', () => {
}
state.queryPoints.push({ ...state.queryPoints[0], id: 2 })
for (const point of store.state.queryPoints) {
state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }))
state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }, true))
}

expect(state.queryPoints.every(point => point.isInitialized)).toBeTruthy()
Expand Down Expand Up @@ -273,17 +273,23 @@ describe('QueryStore', () => {
// initialize all query points so that the store will issue a route request.
state = store.reduce(
state,
new SetPoint({
...state.queryPoints[0],
isInitialized: true,
})
new SetPoint(
{
...state.queryPoints[0],
isInitialized: true,
},
true
)
)
state = store.reduce(
state,
new SetPoint({
...state.queryPoints[1],
isInitialized: true,
})
new SetPoint(
{
...state.queryPoints[1],
isInitialized: true,
},
true
)
)
state = store.reduce(
state,
Expand Down Expand Up @@ -323,6 +329,7 @@ describe('QueryStore', () => {
maxAlternativeRoutes: 1,
points: [],
profile: 'some-profile',
zoom: true,
}
const subRequest: SubRequest = {
state: RequestState.SENT,
Expand Down Expand Up @@ -350,6 +357,7 @@ describe('QueryStore', () => {
maxAlternativeRoutes: 1,
points: [],
profile: 'some-profile',
zoom: true,
}
const subRequest: SubRequest = {
state: RequestState.SENT,
Expand Down
2 changes: 1 addition & 1 deletion test/stores/RouteStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('RouteStore', () => {

describe('clear route information', () => {
it('should revert to initial state on ClearPoints Action', () => {
executeTest(new SetPoint(createEmptyQueryPoint()))
executeTest(new SetPoint(createEmptyQueryPoint(), true))
})
it('should revert to initial state on ClearRoute Action', () => {
executeTest(new ClearRoute())
Expand Down

0 comments on commit 006ed46

Please sign in to comment.