Skip to content

ImageCollection images no longer array when delete element other than last #220

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
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

ImageCollection images no longer array when delete element other than last #220

wants to merge 1 commit into from

Conversation

@MuGGYStyle
Copy link

@MuGGYStyle MuGGYStyle commented Apr 5, 2024

Screenshots

image
image
image
image

@madamebiz madamebiz added the quick win Easy to pick, everybody will be happy. label Apr 5, 2024
@maximehuran maximehuran mentioned this pull request Apr 8, 2024
Open
@maximehuran
Copy link
Member

maximehuran commented Apr 8, 2024

Hi @MuGGYStyle ,

Thank you for your contribution.
We know this case and we are correcting it on Sylius directly using this patch :
Sylius/Sylius#13666
Check https://github.com/cweagans/composer-patches package to see how to apply a patch in your project.

But it does not work since Sylius 1.11 and we haven't got another patch at this moment.
We had a discussion about this issue before on #174 and we have specific case when indexes are not integer as said @jacquesbh in #174 (comment)

@maximehuran maximehuran self-requested a review April 8, 2024 08:11
@madamebiz madamebiz added the Status: needs review Status: needs review label Apr 8, 2024
maximehuran
maximehuran requested changes Apr 8, 2024
View reviewed changes
Copy link
Member

@maximehuran maximehuran left a comment

Choose a reason for hiding this comment

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

The fix does not work for every collection form.

@madamebiz madamebiz added Status: needs update This Issue/PR needs update and removed Status: needs review Status: needs review quick win Easy to pick, everybody will be happy. labels Apr 8, 2024
@maximehuran maximehuran marked this pull request as draft May 25, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maximehuran maximehuran maximehuran requested changes

Assignees

No one assigned

Labels

Status: needs update This Issue/PR needs update

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MuGGYStyle @maximehuran @madamebiz