ModernizacionMuniCBA / Cuanto-tengo

App para conocer el saldo de mi tarjeta RedBus
4 stars 8 forks source link

Añadidos los puntos de recarga y geolocalización #7

Closed audi0slave closed 5 years ago

audi0slave commented 5 years ago

Después de un par de vueltas ahora si logré hacer mas prolijo el PR

Algunas cosas a tener en cuenta:

1- Los cambios realizados están en changelog.txt

2- El API_KEY de google maps (el cual me olvidé de borrar en este commit y lo tuve que actualizar en el otro) tiene que ser reemplazado por uno que sea funcional, el mio es solo de prueba. Hay que tener en cuenta que ahora Google cobra por las solicitudes que se hacen a los mapas. Según veo a red-bus le pasa lo mismo.

3- Algunas funciones dentro de precarga.js las aproveché y las "copié" de la web de red-bus/map.php. Hice las modificaciones necesarias para que se adapte a la necesidad de la app

4- Nunca trabaje con Angular, me puse a estudiar este fin de semana pero me pareció mejor manejarme con lo que mas conozco que fue javascript / jQuery directo. Aún así, no vi que haya una necesidad grande para el mapa de usar angular, pero escucho sugerencias porque soy un neófito en el tema.

5- Vi que les hacía falta la geolocalización, asi que me adelanté y la implementé en este mismo commit. Lo que habría que hacer es cambiar el mensaje de error a mostrarle al usuario en caso que no comparta su ubicación. El mismo está dentro de onGeoError().

audi0slave commented 5 years ago

Acabo de actualizar el branch, no se si está todo bien o si hay algo extra que debo hacer para que entre en el PR.

Por otro lado, algo importante:

Según: https://cordova.apache.org/docs/en/latest/reference/cordova-plugin-geolocation/#installation

Para que Apple no desapruebe la app en el Market, hay que aplicar lo siguiente:

Since iOS 10 it's mandatory to provide an usage description in the info.plist if trying to access privacy-sensitive data. When the system prompts the user to allow access, this usage description string will displayed as part of the permission dialog box, but if you didn't provide the usage description, the app will crash before showing the dialog. Also, Apple will reject apps that access private data but don't provide an usage description.

This plugins requires the following usage description:

NSLocationWhenInUseUsageDescription describes the reason that the app accesses the user's location. To add this entry into the info.plist, you can use the edit-config tag in the config.xml like this:

`

need location access to find things nearby

`

Lo dejo anotado acá. No me tomo el atrevimiento de cambiarlo porque mi config.xml es distinto para poder hacerlo funcionar con Visual Studio.

avdata99 commented 5 years ago

Con respecto a los de Geolocalización e iOS @audi0slave no deberíamos tener problema porque esta app no pide permiso de Geolocalización. Si lo ves incluido quitalo porque no es necesario. Quizás @gastonframirez pueda indicar algo más, entiendo que no necesitamos ese permiso

audi0slave commented 5 years ago

Con respecto a los de Geolocalización e iOS @audi0slave no deberíamos tener problema porque esta app no pide permiso de Geolocalización. Si lo ves incluido quitalo porque no es necesario. Quizás @gastonframirez pueda indicar algo más, entiendo que no necesitamos ese permiso

Si bien es cierto que originalmente no pedía geolocalización, la añadí para poder ubicar al usuario y mostrarle los puntos de recarga cercano, como menciono acá:

5- Vi que les hacía falta la geolocalización, asi que me adelanté y la implementé en este mismo commit. Lo que habría que hacer es cambiar el mensaje de error a mostrarle al usuario en caso que no comparta su ubicación. El mismo está dentro de onGeoError().

Dejo esta imagen para que haya una idea sobre como quedaría, cualquier cosa me decís y lo dejo sin geolocalización. Ningún problema.

image