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

feat(align): refactoriza directiva #273

Merged
merged 2 commits into from
Apr 22, 2021
Merged

feat(align): refactoriza directiva #273

merged 2 commits into from
Apr 22, 2021

Conversation

JSantarelli
Copy link
Contributor

@JSantarelli JSantarelli commented Mar 31, 2021

Modifica directiva para permitir:

  • Alinear elementos en el eje 'Y'
  • Centrar por defecto los elementos en el eje 'X'
  • Setear la altura del contenedor que aloja el elemento a alinear (atributo vheight)

Plus:

  • Se crea archivo directives.scss para agrupar clases de las directivas.
  • Se realiza demo de la directiva en la sección directivas

directiva-aligned

@andrrrl
Copy link
Contributor

andrrrl commented Apr 8, 2021

@JSantarelli! está buenísima la funcionalidad! Lo que sigue lo dejo como comentario porque no estoy seguro que sea para pedir un cambio.
No sé si te estaré matando con esto jaja, pero estaba viendo que como tenemos ya la directiva justify esta directiva debería llamarse align para que siga la misma línea. Si es mucho bardo no pasa nada...
Lo que si, hay que ver que no colisione con esa directiva (justify), ya que la misma tiene esto por defecto:
@HostBinding('class.align-items-center') flex2 = true; o sea cuando aplicamos justify, esta siempre centra verticalmente.

@JSantarelli
Copy link
Contributor Author

JSantarelli commented Apr 9, 2021

Excelente @andrrrl veo de hacer el cambio de nombre, en su momento me generó dudas el hecho de que 'align' ya es un atributo de html y me lo pintaba de rojo al querer utilizarlo (lo mismo pasa en las plex-cards).
Respecto a la colisión con 'justify', es cierto... No sé qué sería lo ideal... supongo que actúen complementándose ambas directivas (como lo hacen las clases css al alinear elementos). El tema es que implicaría refactorizar la directiva justify y asegurarse que no pinche en los lugares en los que se utilizó (y agregar align dado el caso).

@JSantarelli JSantarelli changed the title feat(aligned): refactoriza directiva feat(align): refactoriza directiva Apr 19, 2021
@JSantarelli JSantarelli force-pushed the PLEX-244 branch 2 times, most recently from 960ad78 to 27bf49a Compare April 20, 2021 18:03
@andrrrl
Copy link
Contributor

andrrrl commented Apr 20, 2021

No estoy seguro (1) si es un error, y (2) si tiene que ver con esta directiva en sí, pero noté ese espacio en blanco en el contenedor. Lo dejo sólo como comentario.
image

@JSantarelli
Copy link
Contributor Author

No estoy seguro (1) si es un error, y (2) si tiene que ver con esta directiva en sí, pero noté ese espacio en blanco en el contenedor. Lo dejo sólo como comentario.
image

Gracias @andrrrl, no sería un error, el contenedor de la izquierda acapara el 100% del alto disponible y los otros dos contenedores están haciendo un cálculo con el viewHeight y no coinciden. Igualmente lo atenué un poquito para evitar dolor de ojos.

@andrrrl
Copy link
Contributor

andrrrl commented Apr 21, 2021

No estoy seguro (1) si es un error, y (2) si tiene que ver con esta directiva en sí, pero noté ese espacio en blanco en el contenedor. Lo dejo sólo como comentario.
image

Gracias @andrrrl, no sería un error, el contenedor de la izquierda acapara el 100% del alto disponible y los otros dos contenedores están haciendo un cálculo con el viewHeight y no coinciden. Igualmente lo atenué un poquito para evitar dolor de ojos.

jaja perfecto Julito! queda claro

Copy link
Contributor

@andrrrl andrrrl left a comment

Choose a reason for hiding this comment

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

csselente

@JSantarelli JSantarelli force-pushed the PLEX-244 branch 6 times, most recently from e7aa639 to 73d08df Compare April 22, 2021 11:21
@liquid36
Copy link
Contributor

🎉 This PR is included in version 7.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants