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: issue-340 #627

Merged
merged 3 commits into from
Jun 6, 2024
Merged

Fix: issue-340 #627

merged 3 commits into from
Jun 6, 2024

Conversation

tblivet
Copy link
Contributor

@tblivet tblivet commented May 17, 2024

Questions Answers
Description? Fix #340
Type? improvement
BC breaks? no
Deprecations?
Fixed ticket? Fix #340
Sponsor company @PrestaShopCorp
How to test? ⬇️

Most of the points described in the issue have already been addressed. The remaining ones have been tackled as follows:

  • Enhanced pagination to prevent overflow on mobile devices:
    image
  • Added a Bootstrap breakpoint to provide more flexibility on small devices (use to improve product list on mobile).
  • Updated product list display based on page layouts.
  • Improved mobile display of the product list to feature only one product per row on mobile devices.
    image

tblivet added 2 commits May 17, 2024 17:56
Enhance product list display for various layouts and introduce breakpoints for improved mobile display.
Enhanced pagination and added mobile optimizations.
@tblivet tblivet requested review from nicosomb and ga-devfront May 17, 2024 16:27
nicosomb
nicosomb previously approved these changes May 18, 2024
Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

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

Please keep one column for mobile. In the issue, it was just a suggestion from the author.

@tblivet
Copy link
Contributor Author

tblivet commented May 20, 2024

Please keep one column for mobile. In the issue, it was just a suggestion from the author.

Hi @Hlavtox, I don't really understand, you want me to remove one column for mobile?
I think it's a good optimization as it will concern only devices with screen width < 375. What do you think?

@Hlavtox
Copy link
Contributor

Hlavtox commented May 20, 2024

@tblivet Ah sorry, too early in the morning. :-) I meant to keep two columns on mobiles, unless on VERY small screen sizes. We can fit more data without as much scrolling.

@tblivet
Copy link
Contributor Author

tblivet commented May 20, 2024

@Hlavtox, I think removing it is not a good idea because with 2 products per line on very small devices, we really lose in terms of visibility. As you say, we can fit more data, but we display more data in a bad way.
I suggest keeping this in place but reducing the breakpoint to 360.
With the previous value of 375, the new iPhones will display 2 products per line as it will trigger if < 375. If we lower the value to 360, we will also cover Galaxy, and some other phones, which have a width of 360 for their latest models. This way, one column will only display for small and older devices.
https://gs.statcounter.com/screen-resolution-stats/mobile/worldwide

I think it's a good compromise. What do you think?

@Hlavtox
Copy link
Contributor

Hlavtox commented May 20, 2024

@tblivet Yeah! Good solution. :-) Do you want to move the breakpoint or add another one?

@tblivet
Copy link
Contributor Author

tblivet commented May 20, 2024

@Hlavtox, Perfect 👍 No, just move it because it's already a new breakpoint!

@florine2623 florine2623 self-assigned this Jun 5, 2024
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 @tblivet ,

Thanks for the PR.

When I try on desktop screen, the 3 dots button doesn't work :

Screen.Recording.2024-06-06.at.11.46.12.mov

On mobile 320px size, the pagination design is as expected.

But it doesn't work so well. The 3-dots button doesn't always work.

Screen.Recording.2024-06-06.at.11.47.44.mov

Although this is not blocking, before this PR, it was already like this. It can be fixed later.

The products are well displayed on mobile size.

It is QA ✅

@tblivet
Copy link
Contributor Author

tblivet commented Jun 6, 2024

Thank you, @florine2623 🙏
The three dots are just there to indicate that there are pages between page X and page Y.
They allow us to keep all the pagination on a single line, instead of showing every page. So, it's normal if they do nothing, they're just visual elements.

@tblivet tblivet merged commit 9674ff6 into PrestaShop:develop Jun 6, 2024
6 checks passed
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.

Viewport check: 320px
6 participants