RossiniMaximo / dwf-m8-challengue-front

0 stars 0 forks source link

Review #5

Open alexismunoz1 opened 2 years ago

alexismunoz1 commented 2 years ago

Buenas Maximo, ahora sí, funciona todo como debería, felicitaciones!

Cosas que se podrían mejorar:

Más allá de esos detalles, buen laburo che, el desafío cumple con las de ser aprobado, éxitos en lo que sigue!!

RossiniMaximo commented 2 years ago

Hola Alex , buenísimo muchas gracias. Es verdad las pages me quedaron llenas de lógica, el tema es que no supe cómo manejar esa data sin tener que meterla ahi la verdad, intenté usar los hooks pero no podía porque me tiraba error en el navegador porque como no eran un componente no podía hacer ciertas cosas, y también intente usar selectores para hacer llamadas a la API pero me pasaba que devolvía siempre lo mismo por más que haya sido cambiado , si tenés alguna recomendación es bienvenida jeje, saludos Alex!!

On Mon, Apr 25, 2022, 7:18 PM Alexis Muñoz @.***> wrote:

Buenas Maximo, ahora sí, funciona todo como debería, felicitaciones!

Cosas que se podrían mejorar:

-

Estaría bueno que cuando reportas, editas o reportas info de una mascota, te redirija a la anterior page indicando que todo salió ok

Estaría bueno que en las card de las pets este su ubicación, tanto en "my pets", como en "pets arround"

El componente PopUp.tsx deberia ir en UI, ya que no tiene estado interno, por ende no es un component: [image: Captura de pantalla de 2022-04-25 19-05-51] https://user-images.githubusercontent.com/77214476/165183392-35a659c4-291e-47ae-8797-07f3ea8c9987.png

Las pages están sobrecargadas de lógica, la idea general es que no la tengan y solo llamen componentes, pero también como dijo Marce, son cuestiones que se van puliendo a medida que se va laburando react y se analizan otros proyectos.

Más allá de esos detalles, buen laburo che, el desafío cumple con las de ser aprobado, éxitos en lo que sigue!!

— Reply to this email directly, view it on GitHub https://github.com/RossiniMaximo/dwf-m8-challengue-front/issues/5, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASD4RGIBTPU4EWUF4JSIK4TVG4K2NANCNFSM5UJ5675A . You are receiving this because you are subscribed to this thread.Message ID: @.***>

alexismunoz1 commented 2 years ago

Entiendo a lo que ibas, también intente hacerlo de esa manera y fracase miserablemente jaja Hay una mejor opción y más sencilla, sería sacar toda la lógica de la page, montarla en componentes y llamar el los componentes desde las pages. Te dejo mi repo(que no es ejemplo de nada jaja) en donde lo implemente de esa manera: https://github.com/alexismunoz1/PetsLostApp-Frontend Si miras las pages, no tienen casi nada de logica, solo albergan components. Esto no quiere decir que sea una regla super estricta que hay que respetar a raja tabla, pero si es una buena práctica que permite una buena organización y escalabilidad de proyecto

RossiniMaximo commented 1 year ago

Joya Alex si ahi le estuve dando un ojo y queda mucho mas prolijo la verdad , además que debe ser mas escalable porque tenes todo separado supongo, voy a intentar implementarlo en el siguiente desafio si es que es posible. Muchas gracias x compartir el repo!!

El mar, 26 abr 2022 a las 0:14, Alexis Muñoz @.***>) escribió:

Entiendo a lo que ibas, también intente hacerlo de esa manera y fracase miserablemente jaja Hay una mejor opción y más sencilla, sería sacar toda la lógica de la page, montarla en componentes y llamar el los componentes desde las pages. Te dejo mi repo(que no es ejemplo de nada jaja) en donde lo implemente de esa manera: https://github.com/alexismunoz1/PetsLostApp-Frontend

— Reply to this email directly, view it on GitHub https://github.com/RossiniMaximo/dwf-m8-challengue-front/issues/5#issuecomment-1109257278, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASD4RGLB3VDOMNMFGGBGJGLVG5NRZANCNFSM5UJ5675A . You are receiving this because you commented.Message ID: @.***>