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.

Sign up for GitHub

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

Jump to bottom

Only update the specified target images when an image is edited. #301

Merged
merged 14 commits into from May 12, 2022
Merged

Only update the specified target images when an image is edited. #301

merged 14 commits into from May 12, 2022

Conversation

mitogh
Copy link
Member

@mitogh mitogh commented Apr 16, 2022

Summary

Fixes #299

Relevant technical choices

Allow to select specific target images when editing an image, if either: thumbnail, all except thumbnail or all is selected the edit behavior should only use the specified images instead of applying the edit to all the images regardless of the selected target.

Checklist

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

@mitogh
Add assertions for target type of edits
a3dee98 
When an edit with a target occurrs make sure that only the specified
images are modified, and the metadata reflects this changes based on
the specified target from the media section
@mitogh
Merge branch 'trunk' into feature/update-secondary-image-onedit-with-…
04d1676 
…targets
@mitogh
Merge branch 'trunk' into feature/update-secondary-image-onedit-with-…
a278dad 
…targets
@mitogh
Migrate tests to verify image edits to new file location
7a36d1d 
Move the tests from the single location into the correct
file where the image edits are located.
@mitogh
Handle the scenario when a target is selected.
5ecffc8 
When an image is selected for a target make sure that only
the selected target images are processed instead of processing
all the images when is not required or desired.

Fixes #299
@mitogh mitogh added [Type] Bug An existing feature is broken [Focus] Images Issues related to the Images focus area Needs Review Anything that requires code review [Module] WebP Support Issues for the WebP Support module labels Apr 16, 2022
@mitogh mitogh added this to the First release after 1.0.0 milestone Apr 16, 2022
@mitogh mitogh self-assigned this Apr 16, 2022
@mitogh mitogh changed the title Feature/update secondary image onedit with targets Only update the specified target images when an image is edited. Apr 16, 2022
mitogh and others added 2 commits April 23, 2022 12:42
eugene-manuilov
eugene-manuilov approved these changes Apr 25, 2022
View reviewed changes
Copy link
Contributor

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @mitogh.

felixarntz
felixarntz reviewed May 9, 2022
View reviewed changes
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@mitogh Mostly lgtm, a few minor comments. One more critical thing, the rest is around doc comments.

modules/images/webp-uploads/image-edit.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/image-edit.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/image-edit.php Show resolved Hide resolved
modules/images/webp-uploads/image-edit.php Show resolved Hide resolved
@felixarntz
Copy link
Member

@mitogh @eugene-manuilov I've updated the base branch to release/1.1.0, with the goal to get this into the upcoming release. Would one of you be able to address the final feedback? There's also a merge conflict to address.

@felixarntz felixarntz changed the base branch from trunk to release/1.1.0 May 10, 2022 19:56
@mitogh
Copy link
Member Author

mitogh commented May 10, 2022

Thanks I can take a look at the feedback later today and resolve any remaining conflicts.

@bethanylang
Copy link
Contributor

👋 @mitogh We're in a bit of a rush with this one in prep for Monday's release, so @mehulkaklotar is going to finish this one up. Thank you for continuing to work on this to this point!

@bethanylang bethanylang assigned mehulkaklotar and unassigned mitogh May 11, 2022
mehulkaklotar and others added 3 commits May 12, 2022 13:35
@mehulkaklotar @felixarntz
Update modules/images/webp-uploads/image-edit.php
91427d2 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mehulkaklotar @felixarntz
Update modules/images/webp-uploads/image-edit.php
fb8d988 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mehulkaklotar @felixarntz
Update modules/images/webp-uploads/image-edit.php
d71045f 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mitogh
Copy link
Member Author

mitogh commented May 12, 2022

Sure @bethanylang sorry I was not able to get it done in time.

Thanks, @mehulkaklotar for taking this one to the finish line.

jjgrainger
jjgrainger approved these changes May 12, 2022
View reviewed changes
Copy link
Contributor

@jjgrainger jjgrainger left a comment

Choose a reason for hiding this comment

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

Thanks @mehulkaklotar Latest changes look good to me

felixarntz
felixarntz approved these changes May 12, 2022
View reviewed changes
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @mitogh and @mehulkaklotar!

@felixarntz felixarntz merged commit d39c6aa into release/1.1.0 May 12, 2022
@felixarntz felixarntz deleted the feature/update-secondary-image-onedit-with-targets branch May 12, 2022 16:07
@felixarntz felixarntz mentioned this pull request May 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mehulkaklotar mehulkaklotar mehulkaklotar left review comments

@eugene-manuilov eugene-manuilov eugene-manuilov approved these changes

@jjgrainger jjgrainger jjgrainger approved these changes

@felixarntz felixarntz felixarntz approved these changes

@adamsilverstein adamsilverstein Awaiting requested review from adamsilverstein adamsilverstein is a code owner

@getsource getsource Awaiting requested review from getsource

Assignees

@mehulkaklotar mehulkaklotar

Labels
[Focus] Images Issues related to the Images focus area [Module] WebP Support Issues for the WebP Support module Needs Review Anything that requires code review [Type] Bug An existing feature is broken
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

When an image is edited, perform updates only to the specified target
6 participants
@mitogh @felixarntz @bethanylang @eugene-manuilov @jjgrainger @mehulkaklotar