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 hacked column layout, fix responsivity of sorting #102

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Feb 2, 2023

This should go to 2.0.x, 2.1.x and develop.

Questions Answers
Description? Fixes broken column layout, if you enabled left column on pages, where it wasn't enabled by default. Also fixes responsivity of product list top section - sorting, product count.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket?
Sponsor company
How to test? Play around in responsive mode and see that everything behaves correctly.

How to test

  • Disclaimer - It does not fix broken column layout on some pages, this will be fixed in another PR. Hummingbird has the same issue.
pr_757.webm
  • I would like @hibatallahAouadni to do the QA because she found out this issue.
  • To test best, get 8.1.x branch, install faceted search develop, enable filtering on all pages, enable left column for all pages in BO > Themes > Choose layouts.
  • Then play around with responsive mode on category, manufacturer, search page and others and see that left column now displays correctly on all pages.

@Hlavtox Hlavtox changed the title Fix hacked column layout, fix responsibility of sorting Fix hacked column layout, fix responsivity of sorting Feb 2, 2023
@Hlavtox Hlavtox added the Waiting for QA Status: Waiting for QA feedback label Feb 3, 2023
@hibatallahAouadni hibatallahAouadni self-assigned this Feb 3, 2023
Copy link
Contributor

@hibatallahAouadni hibatallahAouadni 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

Thanks for your PR 🚀

Without filters:

  • Prices drop ✅
  • New Products ✅
  • Best sales ✅
  • Search ❌ 992px-1200px should display 3 products per line
  • Brands ❌ 992px-1200px should display 3 products per line
  • Suppliers ❌ 992px-1200px should display 3 products per line

With filters:

  • Prices drop ✅
  • New Products ✅
  • Best sales ✅
  • Search ❌ the filters aren't displayed
  • Brands ❌ the filters aren't displayed
  • Suppliers ❌ the filters aren't displayed
pr_102.webm

Please check and feedback.

Thanks!

@hibatallahAouadni hibatallahAouadni added Waiting for author and removed Waiting for QA Status: Waiting for QA feedback labels Feb 6, 2023
@Hlavtox
Copy link
Contributor Author

Hlavtox commented Feb 6, 2023

@hibatallahAouadni

Search ❌ 992px-1200px should display 3 products per line
Brands ❌ 992px-1200px should display 3 products per line
Suppliers ❌ 992px-1200px should display 3 products per line

I don't think you read the PR description, it clearly says that it does not fix broken columns on some pages.

Search ❌ the filters aren't displayed
Brands ❌ the filters aren't displayed
Suppliers ❌ the filters aren't displayed

You did not enable left column on these pages in BO > Themes > Choose layouts. It should be obvious that you cannot see filters, if there is no place for them to display.

@hibatallahAouadni hibatallahAouadni added Waiting for QA Status: Waiting for QA feedback and removed Waiting for author labels Feb 6, 2023
Copy link
Contributor

@hibatallahAouadni hibatallahAouadni 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

Thanks for your feedback.
LGTM, QA ✅ 🎉

Thanks!

@hibatallahAouadni hibatallahAouadni added QA ✔️ Status: QA-Approved and removed Waiting for QA Status: Waiting for QA feedback labels Feb 6, 2023
@Progi1984 Progi1984 added this to the 2.0.8 milestone Feb 6, 2023
@Hlavtox Hlavtox merged commit 39e5d6e into PrestaShop:2.0.x Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA ✔️ Status: QA-Approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants