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

Use new way to access category images #654

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Sep 16, 2024

Questions Answers
Description? Changes template to use a new way of accessing category images, prepared in PrestaShop/PrestaShop#32653. After this is merged, we can merge the final part PrestaShop/PrestaShop#36877, tests should work.
Type? refactor
BC breaks? no
Deprecations? no
Fixed ticket?
Sponsor company
How to test? There should be no visible effect in functionality.

How to test

  • Basically, everything should work the same.
  • Check category cover image:
    • Visit a category with a cover image, there should be an image next to category description.
    • Visit a category with NO cover image, there should be NO image next to category description. There should be NO placeholder Image not found image.
  • Check category thumbnail:
    • Assign thumbnail to Men category.
    • Assign NO images to Women category.
    • Visit Clothes. In the subcategories, Men should have an image, Women should have placeholder Image not found image.

@ShaiMagal
Copy link

@florine2623 ZIP build fail is not related. :-)

Copy link

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

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

Hello @Hlavtox ,

Works as expected.

Screenshot 2024-09-17 at 15 03 08 Screenshot 2024-09-17 at 15 03 32

LGTM, it is QA ✅

@Hlavtox Hlavtox merged commit 813304d into PrestaShop:develop Sep 17, 2024
5 of 6 checks passed
@ps-jarvis
Copy link

PR merged, well done!

Message to @PrestaShop/committers: do not forget to milestone it before the merge.

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

Successfully merging this pull request may close these issues.

4 participants