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

Widgets - Image: use code like the one that adds support for Text widget #6630

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

eliorivero
Copy link
Contributor

@eliorivero eliorivero commented Mar 10, 2017

Related #6599

Changes proposed in this Pull Request:

  • make routines for photonization similar to those to enable support in Text widget. Unlike before, where the URL was built, this will parse a <img /> tag just like it would do in content or Text widget and use the resize mode which is more reliable.

Note that this doesn't solve the issue of the poor resampling experienced in Firefox. This seems to be a common occurrence since it also happens with images placed in the Text widget.
The difference between Blink and Gecko is that the zoom parameter isn't sent from Gecko. The srcset attribute isn't built, so the image served is a non-hidpi one.

To the left, you can see how Image and Text widget render in Chrome, and to the right how Firefox does
photon

Testing instructions:

  1. Add a Image widget. Use an image around 1000x1000 for better results. Set image URL and Width/Height to 200/150 respectively.
  2. Add a Text widget and introduce <img src="URL TO SOME IMAGE" width="200" height="150" /> in it.

Proposed changelog entry for your changes:

Widgets - Image: improve rendering of image.

@eliorivero eliorivero added [Feature] Extra Sidebar Widgets [Status] In Progress [Type] Bug When a feature is broken and / or not performing as intended labels Mar 10, 2017
@eliorivero eliorivero added this to the 4.7.1 milestone Mar 10, 2017
@eliorivero eliorivero added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Mar 10, 2017
@eliorivero eliorivero requested a review from jeherve March 10, 2017 15:01
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚢

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Mar 13, 2017
jeherve added a commit that referenced this pull request Mar 14, 2017
@dereksmart dereksmart force-pushed the update/image-widget-photon branch from 4414800 to 41c07fd Compare March 14, 2017 14:52
@dereksmart dereksmart merged commit 5c29b20 into master Mar 14, 2017
@dereksmart dereksmart deleted the update/image-widget-photon branch March 14, 2017 15:09
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 14, 2017
dereksmart pushed a commit that referenced this pull request Mar 14, 2017
@dereksmart
Copy link
Member

merged to 4.7 be85246

dereksmart pushed a commit that referenced this pull request Mar 14, 2017
* Changelog: add #6641

* Changelog: add #6647

* Changelog: add #6631

* Changelog: add #6630

* Changelog: update release date.
dereksmart pushed a commit that referenced this pull request Mar 14, 2017
* Changelog: add #6641

* Changelog: add #6647

* Changelog: add #6631

* Changelog: add #6630

* Changelog: update release date.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extra Sidebar Widgets [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants