Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Take 2: Optimize responsive images for 3/4 and full width templates #771

Merged
merged 2 commits into from
Mar 22, 2016

Conversation

EricRihlmann
Copy link
Contributor

Currently the function foundationpress_adjust_image_sizes_attr is not factoring in several display variables during the creation of <img> tag sizes attribute value. I've updated it to take in account the actual size of the image, the gutters, and the current template.

@EricRihlmann
Copy link
Contributor Author

@olefredrik Travis kicked off this time.

@olefredrik
Copy link
Owner

Thanks, @EricRihlmann !

olefredrik added a commit that referenced this pull request Mar 22, 2016
Take 2: Optimize responsive images for 3/4 and full width templates
@olefredrik olefredrik merged commit ac72881 into olefredrik:master Mar 22, 2016
@Aetles
Copy link
Contributor

Aetles commented Apr 14, 2016

Is it a good idea to hard code these detailed values? It seems very specific (1200, 770 etc), will people understand the need to change these values if they don't use the same page width or if they have a full width page that is not named page-full-width.php?

@EricRihlmann
Copy link
Contributor Author

@Aetles I based this pull request off of how the default Twenty Sixteen theme sets these values. You would also have to hunt down and update the values if you were to modify the CSS or add a new custom template in that theme as well. I don't believe there is currently anywhere in the FoundationPress PHP that stores the corresponding sass breakpoint and gutter values. Have any suggestions?

@Aetles
Copy link
Contributor

Aetles commented Apr 15, 2016

No, really, I haven't really used responsive images in WordPress yet. I was just looking at the code, and thinking about it and if it would introduce issues, being som specific.

On one hand, if it's from Twenty Sixteen then we can assume it is reasonable and tested code. On the other hand, the templates and layout in Twenty Sixteen is probably not as flexible as the Foundation grid is. I'll guess we'll have to wait and see how well it works when people start using it.

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

Successfully merging this pull request may close these issues.

None yet

3 participants