Skip to content

Feature/shelter map with user current location #99

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

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

Conversation

danielbrazrocha
Copy link

🤔 O que foi feito?

  • Adicionado as dependências leaflet e react-leaflet para renderização de mapas
  • Foi adicionado a página https://sos-rs.com/map para renderizar um mapa localizando os abrigos que tenham coordenadas geográficas cadastradas
  • O Pin do abrigo mostra informações como: nome, endereço, se é pet friendly e o status.
  • Há um commit com a lógica toda em um loop no frontend, mas se torna muito custoso (+1MB de dados), mas é uma possibilidade de utilização sem alterações no backend.
  • PR refatorado para utilizar uma nova rota do backend, logo o merge deste PR depende da aprovação da nova rota do backend feat: add shelters/all endpoint to list all shelters brief info and create map of shelters backend#65

📗 Checklist do desenvolvedor
Foi testado localmente? Sim

Screenshots:
Screenshot from 2024-05-11 14-03-55

Screenshot from 2024-05-11 14-03-39

Screenshot from 2024-05-11 14-07-21

👀 Checklist do revisor
Revisor 1️⃣

Você entendeu o propósito desse PR?
Você entendeu o fluxo de negócio?
Você entendeu o que e como foi desenvolvido tecnicamente a solução?
Você analisou se os testes estão cobrindo a maioria dos casos?

🔗 Referência: README com solicitação das funcionalidades abaixo:

  • Adicionar mapa de abrigos: Criar uma tela com um mapa e todos os abrigos.
  • Verificar a posibilidade de usar geolocation para mostrar a posição do usuário no mapa.

@lucianomlima
Copy link
Member

Acho que essa funcionalidade já estava sendo feita no PR #89

@penguinuux
Copy link
Contributor

Acho que essa funcionalidade já estava sendo feita no PR #89

Exatamente. Acredito que podemos aguardar o PR que subi ser aprovado e aproveitar as melhorias visuais implementadas por você. O popup marker e a ideia do botão ficou muito massa!

@danielbrazrocha
Copy link
Author

Quando comecei a codar, o PR não existia ainda, acabamos duplicando algum trabalho.
Creio que a principal dificuldade em criar o mapa é a paginação existente no /shelters.
@penguinuux creio que no seu PR somente esteja pegando os 20 primeiros abrigos e marcando no mapa, correto ou estou errado?
Por isso fiz duas lógicas lá, a primeira num loop batendo 5x no endpoint (aceita maximo de 100) para resgatar todos os 462 abrigos atuais. Mas vêm muita informação desnecessária de supplies e levava um tempo grande para carregar todos.
Depois refatorei para incluir um endpoint específico para isso no backend, PR65 facilitando para o front e diminuindo uso de recursos desnecessários.
Enfim, queria melhorar ainda o Pin dos abrigos com a informação do contato (não padronizada no BD) e um link para a página de cada abrigo...

@penguinuux
Copy link
Contributor

Quando comecei a codar, o PR não existia ainda, acabamos duplicando algum trabalho. Creio que a principal dificuldade em criar o mapa é a paginação existente no /shelters. @penguinuux creio que no seu PR somente esteja pegando os 20 primeiros abrigos e marcando no mapa, correto ou estou errado? Por isso fiz duas lógicas lá, a primeira num loop batendo 5x no endpoint (aceita maximo de 100) para resgatar todos os 462 abrigos atuais. Mas vêm muita informação desnecessária de supplies e levava um tempo grande para carregar todos. Depois refatorei para incluir um endpoint específico para isso no backend, PR65 facilitando para o front e diminuindo uso de recursos desnecessários. Enfim, queria melhorar ainda o Pin dos abrigos com a informação do contato (não padronizada no BD) e um link para a página de cada abrigo...

Daniel realmente acabamos atacando a mesma demanda, porém nossas duas partes podem ser integradas. Em relação aos pins, ele exibia conforme paginado mesmo, e esta sua melhoria será muito importante. Então acredito que por hora é aguardar os PRs subirem e irmos adaptando para estas duas partes conversarem. Tu fez um trabalho muito bom mesmo!

@lucianomlima
Copy link
Member

@penguinuux talvez você possa no seu PR fazer cherry pick dos commits para já deixar tudo num local só.

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.

3 participants