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

Switch article image background-size from cover to contain #155

Closed
wants to merge 1 commit into from

Conversation

medied
Copy link
Contributor

@medied medied commented Feb 25, 2020

Phabricator Link : https://phabricator.wikimedia.org/T244821

Problem Statement

Bigger size images at the article top are sometimes unrecognizable.

Solution

Updating background-size before and after results:

Before After

@medied medied requested a review from hueitan February 25, 2020 00:30
@hueitan
Copy link
Member

hueitan commented Feb 25, 2020

the changes made the lead image to be center, I personally prefer the original version of showing larger image

we will need design to decide how it should look like, some articles are larger in width or height

@hueitan
Copy link
Member

hueitan commented Feb 25, 2020

as per the design, the lead image should be shown in 100% width at the top

@medied
Copy link
Contributor Author

medied commented Feb 25, 2020

Yea I also thought about checking with design, it's probably only a small percentage of images that look unrecognizable with cover because of their size (like that USSR flag).

Another alternative would be setting the size to a specific pixel size, for example. This would be somewhat of a middle point solution, not sure how useful this would be. But again I'd take heed from design making the call. Examples below.

We should remember that either way when a user clicks on the gallery, they will be able to appreciate the image in its full size.

background-size: cover background-size: contain background-size: 400px background-size: 450px

@stephanebisson
Copy link
Collaborator

I think the spirit of the background image is to give the first page "personality", not to show the image is all its glory, we have the gallery for that.

Since the images come in various sizes and ratios, most solutions include some amount of white space and look a bit broken.

@jpita
Copy link
Contributor

jpita commented Mar 3, 2020

what if we use the contain but stick the image to the top of the page?

this way we always show the image above the text

@medied
Copy link
Contributor Author

medied commented Mar 3, 2020

Just heard from Sudhanshu today, he was able to check in and confirm with Carolyn: the design team says we should cancel this PR. Looks like similar conversations have been had before for Android and iOS.

@medied medied closed this Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants