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

Fix ux #9

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Fix ux #9

wants to merge 6 commits into from

Conversation

michalstepniewski
Copy link
Collaborator

No description provided.

Copy link
Member

@vevurka vevurka left a comment

Choose a reason for hiding this comment

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

Please provide description why changes were made - fix UX doesn't mean anything. Please rebase to branch devel, we don't merge to develop.

@@ -76,13 +76,13 @@
{% endif %}
{% endif %}
{% endif %}

<!-- 2017.03.18 not to see recnt edits, find a smarter way
Copy link
Member

Choose a reason for hiding this comment

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

please fix typos.

@@ -19,12 +19,14 @@
geocoder = Geocoder(sources=settings.OMGEO_SETTINGS, postprocessors=[])
geocoder_for_magic_key = Geocoder(
sources=settings.OMGEO_SETTINGS_FOR_MAGIC_KEY)

geocoder_for_magic_key = Geocoder([['omgeo.services.Google', {'settings': {'api_key': 'GST7YMc0AM9UOsEqAYatIS9GOghnYnwZIip_GQypG1c915E7QT9tDFcpOh9bZgKZQoc3YSyaagDIZhkZHn5vHSnvC5N7'}}]])
Copy link
Member

Choose a reason for hiding this comment

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

not pep8 compilant


def _omgeo_candidate_to_dict(candidate, srid=3857):

Copy link
Member

Choose a reason for hiding this comment

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

why new line?

@@ -94,9 +96,38 @@ def geocode(request):
return _geocode_without_magic_key(request, address)


def depolonize(address):
Copy link
Member

Choose a reason for hiding this comment

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

please use Unidecode or more general method to achieve this.

candidates = [candidates[0]]
# if candidates[0].locator_type != 'POI':
# candidates = [candidates[0]]
# print (candidates)
Copy link
Member

Choose a reason for hiding this comment

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

remove print.

@@ -105,8 +136,9 @@ def _geocode_with_magic_key(address, key):
# The exception is a "point of interest" search, where the user's
# chosen suggestion may be a category like "Beaches" and you want to
# see many candidates.
if candidates[0].locator_type != 'POI':
candidates = [candidates[0]]
# if candidates[0].locator_type != 'POI':
Copy link
Member

Choose a reason for hiding this comment

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

why this is commented?

@@ -27,7 +29,7 @@
<div id="map" class="map"
data-has-boundaries="{{ has_boundaries }}"
data-has-polygons="{{ has_polygons }}"
data-bounds="{{ request.instance.map_extent_as_json }}">
data-bounds="{{ request.instance.map_extent_as_json }}"><!--change map_extent_as_json -->
Copy link
Member

Choose a reason for hiding this comment

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

is it TODO or something in comment? If it is please - mark it as TODO.

def _geocode_with_magic_key(address, key):
# See settings.OMGEO_SETTINGS_FOR_MAGIC_KEY for configuration
pq = PlaceQuery(query=address, key=key)
address = depolonize(address)
Copy link
Member

@vevurka vevurka Mar 31, 2017

Choose a reason for hiding this comment

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

why we should depolonize an address actually?

@vevurka vevurka changed the base branch from develop to devel March 31, 2017 15:20
@vevurka vevurka changed the base branch from devel to develop March 31, 2017 15:21
Copy link
Member

@vevurka vevurka left a comment

Choose a reason for hiding this comment

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

Please ensure that your code has a good quality - pep and remove debug messages.

@@ -19,12 +19,14 @@
geocoder = Geocoder(sources=settings.OMGEO_SETTINGS, postprocessors=[])
geocoder_for_magic_key = Geocoder(
sources=settings.OMGEO_SETTINGS_FOR_MAGIC_KEY)

geocoder_for_magic_key = Geocoder([['omgeo.services.Google', {'settings': {'api_key': 'GST7YMc0AM9UOsEqAYatIS9GOghnYnwZIip_GQypG1c915E7QT9tDFcpOh9bZgKZQoc3YSyaagDIZhkZHn5vHSnvC5N7'}}]])
Copy link
Member

Choose a reason for hiding this comment

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

actually, why there is a key hardcoded? it should be in settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants