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

use a sane-sized image here #1125

Merged
merged 1 commit into from Feb 8, 2016
Merged

use a sane-sized image here #1125

merged 1 commit into from Feb 8, 2016

Conversation

aschweigert
Copy link

for http://jira.inn.org/browse/RNS-156

This should not be the full sized image (obviously). The rectangular thumb seems to work but I'd want to test this a bit and make sure that's the best size/crop to use to here

@aschweigert aschweigert added status: needs review priority: high Either blocks work on a priority-normal task or a solution here informs other work. labels Feb 7, 2016
@aschweigert aschweigert added this to the 0.5.5 - Story Elements milestone Feb 7, 2016
@benlk
Copy link
Collaborator

benlk commented Feb 8, 2016

rect_thumb is max 800x600, and crops to fix the 4:3 aspect ratio.

The two oddly-large images I see on that site's news category are:

  • /wp-content/uploads/2015/09/RNS-MUSLIM-AIRLINE080815.jpg, which is resized to 800x600
  • /wp-content/uploads/2015/09/thumbRNS-KENTUCKY-CLERK090315a.jpg, which is resized to 533x600

I tested this on vagrant by uploading those two images (the unresized versions linked above) to a site already running this Largo, and while the image for the homepage featured image remained oddly large, visually, it wasn't full-resolution, so that's good. This PR succeeds.

We should make this image smaller: https://github.com/INN/Largo/blob/master/partials/archive-category-secondary-feature.php#L5 It never gets wider than 273px in desktop or mobile views, so we can easily use a smaller-sized thumbnail there.

@benlk
Copy link
Collaborator

benlk commented Feb 8, 2016

Using http://nonprofitquarterly-org.largoproject.staging.wpengine.com/category/management/ as a test, this looks good to go.

benlk added a commit that referenced this pull request Feb 8, 2016
@benlk benlk merged commit 983ef47 into develop Feb 8, 2016
@aschweigert
Copy link
Author

the content partial is also used in the river on the homepage which means (I think) that it will sometimes be as wide as 768px so 800px probably does make sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants