FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix dominant color upload process to not override potential third-party editors by spacedmonkey · Pull Request #401 · WordPress/performance · GitHub

/ performance Public

Fix dominant color upload process to not override potential third-party editors - #401

Merged
spacedmonkey merged 4 commits into
trunkfrom
fix/third-party-editor
Jul 6, 2022
ConversationCommits (4)ChecksFiles changed
Merged

Fix dominant color upload process to not override potential third-party editors#401
spacedmonkey merged 4 commits into
trunkfrom
fix/third-party-editor

Conversation

spacedmonkey commented Jun 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Replace editor in a cleaner way.

Fixes #374

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

spacedmonkey added [Type] Bug An existing feature is broken [Focus] Images [Plugin] Image Placeholders Issues for the Image Placeholders plugin (formerly Dominant Color Images) labels Jun 28, 2022
spacedmonkey requested a review from felixarntz June 28, 2022 21:44
spacedmonkey self-assigned this Jun 28, 2022
spacedmonkey requested a review from pbearne as a code owner June 28, 2022 21:44
pbearne approved these changes Jun 29, 2022
View reviewed changes

pbearne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nice

felixarntz added this to the 1.3.0 milestone Jun 29, 2022
felixarntz changed the title Dominant color upload process ignores potential third-party editors Fix dominant color upload process to not override potential third-party editors Jun 29, 2022
felixarntz reviewed Jun 29, 2022
View reviewed changes

felixarntz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@spacedmonkey Thanks, production code looks good, just one tiny nit-pick.

There are unit test failures though, could you check and address those?

Comment thread modules/images/dominant-color/load.php Outdated
spacedmonkey and others added 2 commits June 29, 2022 19:13
Apply suggestions from code review
31972dc 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Gifs are all transparent.
cdc5690

spacedmonkey commented Jun 29, 2022

Copy link
Copy Markdown
Member Author

There are unit test failures though, could you check and address those?

Tests are failing because #405. This is because ImageMagik is flagging all gifs as transparent. I have put a workaround in for now and we can tackle that issue there.

spacedmonkey requested a review from felixarntz July 6, 2022 15:04
adamsilverstein approved these changes Jul 6, 2022
View reviewed changes

adamsilverstein left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Code looks good to me!

spacedmonkey merged commit 525209d into trunk Jul 6, 2022
spacedmonkey deleted the fix/third-party-editor branch July 6, 2022 15:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

pbearne pbearne approved these changes
adamsilverstein adamsilverstein approved these changes
felixarntz Awaiting requested review from felixarntz

Assignees

spacedmonkey

Labels

[Plugin] Image Placeholders Issues for the Image Placeholders plugin (formerly Dominant Color Images) [Type] Bug An existing feature is broken

Projects

None yet

Milestone

1.3.0

Development

Successfully merging this pull request may close these issues.

Dominant color upload process ignores potential third-party editors

5 participants

Back | FazBrowse Home | New Git URL