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.

Sign up for GitHub

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

Jump to bottom

Change Store/Group location by clicking on map #391 #441

Merged
merged 3 commits into from May 11, 2017
Merged

Change Store/Group location by clicking on map #391 #441

merged 3 commits into from May 11, 2017

Conversation

D0nPiano
Copy link
Contributor

Closes #391

@codecov-io
Copy link

codecov-io commented Apr 10, 2017 •
edited

Codecov Report

Merging #441 into master will decrease coverage by 0.12%.
The diff coverage is 78.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
- Coverage    98.5%   98.37%   -0.13%     
==========================================
  Files         217      217              
  Lines        3008     3020      +12     
==========================================
+ Hits         2963     2971       +8     
- Misses         45       49       +4
Impacted Files Coverage Δ
.../store/_storeEditCreateForm/storeEditCreateForm.js 100% <ø> (ø) ⬆️
...p/_groupEditCreateForm/groupEditCreateForm.spec.js 100% <100%> (ø) ⬆️
...e/_storeEditCreateForm/storeEditCreateForm.spec.js 100% <100%> (ø) ⬆️
...upEditCreateForm/groupEditCreateForm.controller.js 81.48% <75%> (-4.24%) ⬇️
...reEditCreateForm/storeEditCreateForm.controller.js 88% <77.77%> (-6.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfc0824...e4b2cd3. Read the comment docs.

D0nPiano
D0nPiano commented Apr 10, 2017
View reviewed changes
client/app/app.js
@@ -70,6 +70,10 @@ angular.module("app", [
.run(AppLocalizeRun)
.config(AppMaterial)
.config(AppHTTPErrorHandler)
.config(( $mdGestureProvider ) => {
Copy link
Contributor Author

@D0nPiano D0nPiano Apr 10, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure if tinkering with the mdGestureProvider is a good idea - but otherwise clicks are not working on mobile maps. Any good ideas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there documentation on this? I found https://github.com/angular/code.material.angularjs.org/blob/master/HEAD/partials/api/material.core.gestures/service/$mdGestureProvider.html

I guess the touch event is intercepted by $mdGesture and does not reach leaflet. But is there another event emitted that could be used by leaflet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I found angular/material#1300 (comment) - so apperently this destorys certain swipe features of angular material, but I think having a working map is more important. Will not only be interesting for this issue, but also for clickable popups etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we can think about this more when we need md-swipe.

@djahnie
Copy link
Member

djahnie commented Apr 10, 2017

Ooooh amazing! 😃
Thx @D0nPiano for tackling this! 😗

@djahnie djahnie closed this Apr 10, 2017
@djahnie djahnie reopened this Apr 10, 2017
tiltec
tiltec reviewed Apr 14, 2017
View reviewed changes
client/app/components/group/store/_storeEditCreateForm/storeEditCreateForm.controller.js Outdated
longitude: e.leafletEvent.latlng.lng
};
$scope.mapController.marker = {
p: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeated code, maybe move it into a function?

tiltec
tiltec reviewed Apr 14, 2017
View reviewed changes
client/app/components/group/store/_storeEditCreateForm/storeEditCreateForm.controller.js Outdated
}
});

$scope.mapController = this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? this is already bound to $scope.$ctrl.

@tiltec tiltec changed the title Change Store location by clicking on map #391 Change Store/Group location by clicking on map #391 May 11, 2017
@tiltec tiltec merged commit 41b93ee into master May 11, 2017
@tiltec tiltec deleted the addEditstoreMap#391 branch May 11, 2017 14:14
tiltec pushed a commit that referenced this pull request May 6, 2018
@nicksellen @tiltec
Replace usages of setUpClass(cls) with setUp(self) (#441)
9e796c5
tiltec pushed a commit that referenced this pull request May 6, 2018
@tiltec
Refactor stores app (#450)
0ab3abf 
Closes #368 

* Move PickupDate, PickupDateSeries and Feedback models to new pickups app

* Bugfixes

* Data migration

* Minor code style improvements

* PEP8

* Add migration test

* PEP8

* Fix merge commit af12da1

* Remove a 'classmethod' decorator that survived #441
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiltec tiltec tiltec left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@D0nPiano @codecov-io @djahnie @tiltec