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

Fix FormSubmitButton selector #27228

Merged
merged 1 commit into from
Feb 17, 2022
Merged

Conversation

atomiix
Copy link
Contributor

@atomiix atomiix commented Jan 5, 2022

Questions Answers
Branch? develop
Description? Since #24791, we use the arrow function (() => {}) for $(document).on() which give this the instance of the class FormSubmitButton and not the selector like before. This PR fixes it by using event.target instead.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #26985 & Fixes #27311
How to test? Please see #26985
Possible impacts? This should have a (positive) impact where FormSubmitButton is used:
  • The button to delete the cover image of category in Sell > Catalog > Categories
  • The button to delete the menu thumbnails image of category in Sell > Catalog > Categories

This change is Reviewable

@atomiix atomiix requested a review from a team as a code owner January 5, 2022 18:28
@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix labels Jan 5, 2022
@atomiix atomiix added this to the 8.0.0 milestone Jan 5, 2022
@jolelievre jolelievre added the Waiting for QA Status: action required, waiting for test feedback label Jan 6, 2022
Copy link

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

Works, you can also use a function instead of an arrow function if the scope is fine

@HanaRebaiQA HanaRebaiQA self-assigned this Jan 6, 2022
@HanaRebaiQA
Copy link

HanaRebaiQA commented Jan 11, 2022

Hello @atomiix

I have tested this PR, but the test is blocked because of these issues : #27237 and this #27303
So, will check again after the fix of the issues.

Thanks!

@HanaRebaiQA HanaRebaiQA added the Blocked Status: The issue is blocked by another task label Jan 11, 2022
@HanaRebaiQA HanaRebaiQA removed the Blocked Status: The issue is blocked by another task label Feb 4, 2022
Copy link

@HanaRebaiQA HanaRebaiQA left a comment

Choose a reason for hiding this comment

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

Hello @atomiix

I checked again this PR, but it still blocked because of this issue.
It will be tested again when the issue will be fixed.

Thanks!

@HanaRebaiQA HanaRebaiQA added the Blocked Status: The issue is blocked by another task label Feb 8, 2022
@HanaRebaiQA HanaRebaiQA removed the Blocked Status: The issue is blocked by another task label Feb 16, 2022
Copy link

@HanaRebaiQA HanaRebaiQA left a comment

Choose a reason for hiding this comment

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

Hello @atomiix

Finally, i have tested this PR after being blocked.
I checked all the related issues, they are fixed.
I checked with multistore, different image format, different langauges (as RTL) and different browsers (Chrome & Firefox)

PR27228.mp4
firefox.mp4
multistore.+.all.format.images.+.FO.result.mp4
RTL.languages.mp4

So, QA ✔️

Thanks!

@HanaRebaiQA HanaRebaiQA added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Feb 17, 2022
@Progi1984 Progi1984 merged commit 5838a88 into PrestaShop:develop Feb 17, 2022
@Progi1984
Copy link
Member

Thanks @atomiix & @HanaRebaiQA

@atomiix atomiix deleted the fix/26985 branch February 17, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug fix Type: Bug fix develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
8 participants