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

Adding support for Discover's fullbleed class. #2942

Merged
merged 2 commits into from
Feb 2, 2016

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Feb 1, 2016

When Discover editors apply a fullbleed class to images on discover.wordpress.com, we extend the images outside the boundaries of the text column. We’d like to bring this behavior to the Reader as well.

Extends the width by 175px in each direction, aligning with the Discover sidenotes from PR #2729. Larger viewports only: no change to screens under 1132px.

Examples:
Article: https://discover.wordpress.com/2015/12/28/rob-turpin-interview/
Reader: http://calypso.localhost:3000/read/post/id/53424024/1661
example1

Article: https://discover.wordpress.com/2016/01/21/fennabee/
Reader: http://calypso.localhost:3000/read/post/id/53424024/9037
example2

Lets big images break out of the frame if the editors choose for them
to.
@kjellr kjellr self-assigned this Feb 1, 2016
@kjellr kjellr added [Feature] Reader The reader site on Calypso. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 1, 2016
@kjellr
Copy link
Contributor Author

kjellr commented Feb 1, 2016

@bluefuton It looks like we cap the size of images at 1440px wide ( ?w=1440&quality=80&strip=info ) inside the full-post views: any chance we could bump that up to 2164px for Discover images/captions with a fullbleed class? That'd keep these ones nicely retinafied. :)

@blowery
Copy link
Contributor

blowery commented Feb 1, 2016

@kjellr we can probably do that. nice work!

Also fix the resize-image-url function to understand high DPI screens.
@blowery
Copy link
Contributor

blowery commented Feb 1, 2016

There we go, that should do it. 37052c3

@kjellr
Copy link
Contributor Author

kjellr commented Feb 1, 2016

Awesome, thanks @blowery! Tested it out on a couple additional posts and it looks great!

blowery added a commit that referenced this pull request Feb 2, 2016
Adding support for Discover's fullbleed class.
@blowery blowery merged commit 9fa0f28 into master Feb 2, 2016
@blowery blowery deleted the add/reader-discover-fullbleed branch February 2, 2016 15:16
@jancavan jancavan removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Reader The reader site on Calypso.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants