adobe / asset-share-commons

A modern, open-source asset share reference implementation built on Adobe Experience Manager (AEM)
https://opensource.adobe.com/asset-share-commons/
Apache License 2.0
88 stars 107 forks source link

Portrait mode assets are cropped in the asset share page #84

Open ar80001-BRONGA opened 6 years ago

ar80001-BRONGA commented 6 years ago

Portrait mode assets are cropped such that only their centers are shown within the asset share page.

Card layout assetsharecardlayout

List layout assetsharelistlayout

davidjgonzalez commented 6 years ago

Correct - do you have a better universal solution?

ar80001-BRONGA commented 6 years ago

I will try a few things out and suggest a solution if it handles both portrait and landscape images properly. Thanks for the quick response!

davidjgonzalez commented 6 years ago

Happy to incorporate - this was the hang that seemed to look best across sizes that we found. We didn’t like variable height cards since they tended to make uneven rows which are hard to scan.

Something we could do as well is add a “thumbnail style” to the results components that supports a few different approaches so users can choose if it’s more important to them.

davidjgonzalez commented 6 years ago

Hang = best (not edit of comment on phone ;))

ar80001-BRONGA commented 6 years ago

I looked into this, and just changing the object-position property from "center" to "top" improves things quite a bit.

Card layout Updated the object-position property from the .ui.cards > .card > .image > img, .ui.card > .image > img style. assetsharecardlayoutfix

List layout Updated the object-position property from the .ui.table tbody tr td.image:not(.ui) img style. assetsharelistlayoutfix

I'm not sure if this change will have implications anywhere else, but it looks pretty good in my local AEM 6.3 SP1 development environment. :-)

davidjgonzalez commented 6 years ago

Wonder if this should be configurable... i guess this really depends on the composition of the image. position top looks funny for things that have less content at the top of the image (of course)

image

image

ar80001-BRONGA commented 6 years ago

I totally agree about images with less content at the top, and this begs an interesting question my manager had...would it make sense to allocate more of the card real estate to the image and less to the title, data and actions? If this were a viable option it might mitigate the issue if the object-position property were changed from "center" to "top"...thoughts?

godanny86 commented 6 years ago

yes that could be a good enhancement to offer a few different thumbnail size options. Not sure if you have seen this @artdirk: https://adobe-marketing-cloud.github.io/asset-share-commons/pages/development/guide/ -> Extend Search Results Renderer. Its fairly easy to create a custom Card / List renderer to add/modify the metadata displayed but could also be used to change the size of the card thumbnail.

davidjgonzalez commented 6 years ago

To allow authors to configure the size.. im wondering how that might look since since we don't have a way to "Author" different views; you just pick the renderer.

image

It seems like the most reasonable option would be to add 2 more options to the above that are

Thumbnail size, Thumbnail position (top, center, bottom, cover, etc.)

Size gets interesting since we can't do pixels since the card and list views thumbs wont be the same height; so we could do a % off whatever is the "default heights" (50%, 100%, 200%, 500% etc) or we could create some arbitrary list of sizes (Small, Medium, Large) and each is set to some predefined height (Not sure what granularity of sizes would make everyone happy - if that's possible)