contao / contao-manager

Contao Manager
GNU Lesser General Public License v3.0
83 stars 33 forks source link

Align package details vertically centered #814

Closed lukasbableck closed 3 days ago

lukasbableck commented 2 weeks ago

This PR fixes the uneven top and bottom gaps for the .package__details element.

Before:

image

After:

image
zoglo commented 2 weeks ago

Wouldn't work in this case: image

The container itself would need to be wrapped.

lukasbableck commented 2 weeks ago

True, or we just apply this if >1024px 😄

zoglo commented 2 weeks ago

@lukasbableck wouldn't an align-self: center; on the .package-details do the same since the parent is already a flex container that stretches the items 😉(You'd be able to drop the media query and there is no need changing it to flex completely).

lukasbableck commented 2 weeks ago

Not quite... This would not center the .package__about which looks kinda weird IMO.

image

And apparantly this also doesn't fully work if the .packageabout is taller than the .packagerelease and .package__actions.

zoglo commented 2 weeks ago

image

I still feel like that the align-self version would be better towards the image - we are talking about "pixel-perfect" design right now and the d-flex centered version would still bother someone that'll compare it to the height of an image 🤷

lukasbableck commented 2 weeks ago

The image height currently doesn't match anyway, it just happens to kinda (not exactly) visually fit with the Contao logo because that one is not square. For something like contao-rocksolid-icon-picker which provides a square icon, the height doesn't match with any of those options. So maybe there is more to change?

orig align-self pr
zoglo commented 2 weeks ago

If you change the scope of your PR and rename it, you may want to match the styles with the ones in this commit: https://github.com/contao/package-list/commit/ba6b2a996d9272fa63b6013979a36a29d03330cc.

The icons were resized to 100px in the package list and aren't centered on landscape to show the logo. That may give the package-details just enough height that the container wouldn't need to be centered as well.

What if we'd align-self AND center with your version at 1024px? The container itself needs to be centered, so do the items as seen in the screenshots.

aschempp commented 1 week ago

Shouldn't this be fixed in the package-list itself (instead of the Contao Manager)?

zoglo commented 1 week ago

@aschempp that's not part of the package-list, it's the installed packages, they aren't really related (right now)

zoglo commented 3 days ago

Screenshots of the current PR @aschempp

Mobile image

< 1024px image

>=1024px image

aschempp commented 3 days ago

Thank you @lukasbableck