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

añadir geolocalizacion por google #61

Open
wants to merge 1 commit into
base: miguel-nasaRoverChallenge
Choose a base branch
from

Conversation

gargonmi
Copy link
Collaborator

Geolocalizacion con google (quiera o no el usuario)

@gargonmi gargonmi requested a review from UlisesGascon January 18, 2019 11:16
@UlisesGascon UlisesGascon mentioned this pull request Jan 23, 2019
46 tasks
Copy link
Contributor

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

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

Sería bueno hacer un refactor en profundidad, me gusta mucho la idea del fallback con localización por IP! ;-)




let posicion = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

No necesitas esta variable global...



function geo_success(position) {
posicion.latitud = parseFloat(position.coords.latitude.toFixed(4));
Copy link
Contributor

Choose a reason for hiding this comment

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

Podrías meter esta transformación parseFloat(position.coords.latitude.toFixed(4)); en una función.. para reutilizarla después

function geo_success(position) {
posicion.latitud = parseFloat(position.coords.latitude.toFixed(4));
posicion.longitud = parseFloat(position.coords.longitude.toFixed(4));
initMap(posicion.latitud,posicion.longitud);
Copy link
Contributor

Choose a reason for hiding this comment

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

No te hace falta poSicion si ya tienes posiTion

initMap(posicion.latitud,posicion.longitud);
}

function geo_error() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Esto parece un poco poltergeist


//probando la geolocalizacion
function searchMe(){
navigator.geolocation.getCurrentPosition(geo_success, geo_error);
Copy link
Contributor

Choose a reason for hiding this comment

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

geo_error no hace más que ejecutar... altSearch, pásalo directamente ;-)

let marker;

function initMap(long,lat) {
map = new google.maps.Map(map, {
Copy link
Contributor

Choose a reason for hiding this comment

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

mala idea reutilziar una variable map que era un selector para que luego sea otro objeto. 😢


function initMap(long,lat) {
map = new google.maps.Map(map, {
center: {lat: long, lng: lat},
Copy link
Contributor

Choose a reason for hiding this comment

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

Cómo es que lat es long ???

Nota: con es6 puedes dejarlo en center: {lat, lng},

map = new google.maps.Map(map, {
center: {lat: long, lng: lat},
zoom: 14,
styles:[
Copy link
Contributor

Choose a reason for hiding this comment

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

Este array podría estar expuesto en otro fichero como style_maps.js o similar...

<div style="height: 600px" id="map"></div>

</div>
<script src="whereIAm.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Tu script siempre va al final de las dependencias..


</div>
<script src="whereIAm.js"></script>
<script src="https://maps.googleapis.com/maps/api/js?key=AIzaSyB0AeWwQW28gYeltyIMsTRYGh-Zu3YEYLQ&callback=searchMe"
Copy link
Contributor

Choose a reason for hiding this comment

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

necesitabamos bootstrap?

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