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

BO: Fixed position with new slide and remove check on non present input position #23

Merged
merged 3 commits into from
May 28, 2020

Conversation

beyondsagency
Copy link

This is a fix for ps_imageslider whe we add a new slide the position is not good and we have function getNextPosition() not use, so use it :)

Beyonds, Digital Agency

Copy link
Member

@Quetzacoalt91 Quetzacoalt91 left a comment

Choose a reason for hiding this comment

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

Hmm, I wonder if we won't have a broken with this PR. It seems this method is called when we try to reorder slides, how will the position be taken in account?

ps_imageslider.php Outdated Show resolved Hide resolved
@beyondsagency
Copy link
Author

You're right, this method need to stay public. And about new slide added, get the next position with my commit ?

@Progi1984
Copy link
Member

Ping @Quetzacoalt91 to @beyondsagency question ;)

Copy link
Member

@Quetzacoalt91 Quetzacoalt91 left a comment

Choose a reason for hiding this comment

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

I totally missed it, sorry. It now looks good to me.

@Progi1984
Copy link
Member

Ping @beyondsagency for answer ;)

@beyondsagency
Copy link
Author

OK can we merge ?

@Progi1984
Copy link
Member

@beyondsagency Sorry, I forgot to send QA ;)

@sarahdib sarahdib self-assigned this May 28, 2020
@Progi1984 Progi1984 merged commit 3ee2935 into PrestaShop:dev May 28, 2020
@Progi1984
Copy link
Member

Thanks @beyondsagency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants