luissrl / project1

Proyecto Módulo 1
0 stars 0 forks source link

Detalles #1

Open angabar opened 5 years ago

angabar commented 5 years ago

Buenas tardes Luis:

Acabo de revisar tu proyecto y si que hay un par de detalles a comentar para que los tengas en cuenta mientras realizas el proyecto:

Márgenes molestos

Por defecto los navegadores aplican unos margin a nuestras plantillas HTML que puede resultar un poco molesta, si ves en tu proyecto sale como un pequeño espacio blanco entre el navegador y la web, esto se corrige usando el selector universal y poniendo todos los margin a 0:

* {
  margin: 0px;
}

Encabezado

He visto que el encabezado lo has dispuesto con una etiqueta img y otras para los textos, está bien, pero hay una forma mejor. Como el encabezado del proyecto tiene un texto en el medio, es mejor usar un div que tenga una imagen de fondo usando el background-image: url(...) y ajustarlo con las propiedades background-size background-position:

background-image: url(https://picsum.photos/id/350/1200/1200);
background-position: center;
background-size: cover;

De esta forma el manejo del texto te va a salir mucho mejor ya verás.

El porque

Si quieres una regla de oro para saber cuando usar una img o un background-image piensa solo en si es una imagen con elementos por encima, siendo elementos un botón, texto... cualquier cosa.

Solo usaremos las etiquetas img cuando queramos poner la imagen tal cual, sin nada por encima, si vemos que esta va a tener un texto o demás cosas entonces es mejor usar un div con un background-image.

Sigue así Luis, vas muy bien!

luissrl commented 5 years ago

Hola,

gracias! Cierto, lo de los márgenes se me pasó completamente, y lo de la imagen me volví un poco loco.

Por cierto, en el CSS hay unos comentarios acerca del .transform. no acabo de entender muy bien su uso. Lo hice consultando en la web, pero no lo acabo de ver claro del todo.

Ahora voy a por las Card, que también me están dando trabajazo!!! jeje.

Gracias!

Saludos

angabar commented 5 years ago

Buenos días Luis:

Tras estudiar detenidamente el proyecto he podido ver algunas cosas que quería comentarte, son detalles más que nada, el proyecto en lineas generales está quedando muy bien:

Cierre correcto de las etiquetas

Lo primero que me ha llamado la atención es que hay una etiqueta div en la "card2" y la "card3" que no está bien cerrada, lo que hace que el prettier no funcione correctamente y no puedas formatear el código, quedate con la premisa de que si no podemos formatear el código con prettier es porque tenemos una etiqueta sin cerrar de esta forma podemos detectar a tiempo los errores en HTML. Es importante cerrar todo bien ya que aunque ahora no de errores, cuando pasemos a Angular si que tendremos fallos en la aplicación. Bien, mira, el error está en las líneas 40 y 50, tienes algo como esto:

Línea 40:
<div class="titulo_card2"></div><h3>Card title 2</h3></div>
Línea 50:
<div class="titulo_card3"></div><h3>Card title 3</h3></div>

Si te fijas bien podrás apreciar que el último div no tiene etiqueta de apertura, lo que hace que todo el documento HTML se des-estructure. Yo eliminaría esa última etiqueta de cierre que sobra y pondría los h3 dentro del contenedor "titulo_card" que imagino que tendría que guardar el título.

Recuerda formatear el código para ver si está todo OK.

Nombres de las clases

Otra cosa, antes de pasar a las dudas de CSS es que he visto que usas nombres de clases diferentes para hacer referencia a elementos totalmente iguales, mira, en las líneas 28, 36 y 46 tienes estos códigos:

Línea 28:
<div class="card1">
Línea 36:
<div class="card2">
Línea 46:
<div class="card3">

A ver, está bien, no da error, pero piensa que en el caso de que tengamos 50 cards o incluso más, vamos a tener que ir numerando todas ellas, lo cual seria un trabajo innecesario. Ten como premisa que si un elemento está repetido lo suyo es ponerle el mismo nombre de la clase de esta forma en un único lugar modificamos todos los elementos a la vez.

Duda CSS 1:

La primera duda que veo en el código es:

/* Asi no funciona correctamente.....??
.card1, .card2, .card3 {
  height: 400px;
  width: 350px;
} */

Pero no acabo de entender bien, porque a mi si que me funciona ese código, eso si, el HTML tiene que estar bien cerrado, prueba a corregir el HTML y si el error persiste lo volvemos a ver, vale? ;)

Como te decía en el punto anterior, si pones una clase "card" común a todas las tarjetas entonces simplemente tienes que poner .card {..} y estos estilo aplicarán a todas las tarjetas independientemente de si tenemos 3 o 40.

Duda CSS 2:

La siguiente duda que veo en el código es:

transform: translate(-50%, -50%);

Bien, esto es porgue has usado posicionamiento absoluto y la única forma de centrar elementos con posicionamiento absoluto es usando el transform, me explico:

Cuando nosotros posicionamos algo con posicionamiento absoluto, tenemos que darle una coordenadas como has hecho tu al poner top: 25% y left: 50% el problema que radica de utilizar esta metodología está en que el left hace que el elemento quede demasiado a la derecha:

ejemplo1

Como ves el elemento no está centrado, es por eso que tenemos que utilizar la propiedad transform para desplazar la mitad de lo que ocupa el elemento a la izquierda y la mitad de su altura hacia arriba para eso sirve el transform. Es un poco complejo de ver, así que si lo deseas podemos hacer una tutoría y te explico mejor el concepto.

Pero te voy a decir algo que no te gustará, quiero que elimines el posicionamiento absoluto, porque aunque está bien, tenemos soluciones como flex que pueden ayudarnos mucho mejor a centrar elementos como los textos del encabezado.

Una vez haya borrado el posicionamiento de los textos puedes poner los textos en un div que englobe a ambos:

<div class="cabecera">
  <div class="my_text">
    <div class="centered"><h2>My web</h2></div>
    <div class="centered2"><h3>Proyecto módulo 1</h3></div>
  </div>
</div>

Ahora solo tenemos que utilizar flex en el contenedor padre "cabecera":

.cabecera {
  dispaly: flex;
  justify-content: center;
  align-items: center;
}

Esta solución es mucho mejor ya que es más fácil de mantener que las posiciones absolutas, sino te sale el código o sigues teniendo alguna duda, házmelo saber ;)

Va genial Luis, tan solo son unos cuantos puntos a tener en cuenta, pero vas pillando muy bien los elementos HTML y sus propiedades de CSS, animo y sigue así!

Si tienes alguna duda más me comentas por aquí o concretamos una tutoría, lo que tu prefieras. A por todas!

luissrl commented 5 years ago

Hola,

Gracias!!!! estuve 1 hora!! buscando porque no podía formatear el texto, y no me daba cuenta que había etiquetas mal. Al no saber de eso, tampoco conseguía entender el porque.

Perfecto a lo del fondo con el texto con flex, mucho más fácil la verdad.

Aunque me falta acabar las card, tengo dudas con los elementos del navbar, Mejor div's o mejor una lista?

Algo mejorable así de un vistazo rápido? se me ha ocurrido echar un vistazo al github de Iván, y no se el resultado, pero vaya nivel hay ahí :( si lo se no miro!

Voy a seguir a ver si finalizo las card, pruebo en otro tamaño de pantalla, y hago pruebas con el menú.

Feliz finde!

Un abrazo

angabar commented 5 years ago

Buenas tardes Luis!

Vamos con esas dudas:

Aunque me falta acabar las card, tengo dudas con los elementos del navbar, Mejor div's o mejor una lista?

Para mi está genial como lo has hecho tu, el navbar yo lo dejaría así, se pueden usar los li pero es más manipulable un div así que para mi es mejor tu elección ;)

Algo mejorable así de un vistazo rápido?

Tienes un scroll horizontal que te sale en la pantalla debido a que el contenedor con la clase "container" tiene un padding que hace que el elemento ocupe más de ese 100% que le especificas tu, recuerda que si ponemos un padding el elemento ocupará más de lo especificado, para solucionarlo simplemente aplícale un box-sizing: border-box; a la clase "container".

Puedes también intentar poner un border-radius a las cards así como a las imágenes así como hacer que el elemento "Leer mas" se quede a la derecha con un pequeño espacio entre este y lo que sería el borde de la card. Lo demás está perfecto, enhorabuena!

se me ha ocurrido echar un vistazo al github de Iván, y no se el resultado, pero vaya nivel hay ahí :( si lo se no miro!

Iván ya tiene amplios conocimientos de HTML y CSS ya que como pone en la presentación es diseñador gráfico, así que parte con ventaja, vas bien, te lo digo yo, no te preocupes y sigue esforzándote como hasta ahora, ya verás en Marzo las cosas que eres capaz de hacer ;)

Buen finde Luis y si te quedas con alguna duda mas coméntame!

luissrl commented 5 years ago

Hola Enric,

he hecho lo del box-sizing, pero sigue saliéndome el scroll y no detecto donde está el error, voy a revisar que no haya algún padding más.

Saludos