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 product page overflow in mobile view #383

Merged
merged 4 commits into from
Dec 6, 2022
Merged

Conversation

lucrp
Copy link
Contributor

@lucrp lucrp commented Oct 27, 2022

Questions Answers
Description? In product.tpl there is a g-5 class in <div class="row g-5 product js-product-container"> that causes a slight overflow. Changing it to g-lg-5 fixes that.
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #382
How to test? Mobile view in product page
Possible impacts? Please indicate what parts of the software we need to check to make sure everything is alright.

@Hlavtox
Copy link
Contributor

Hlavtox commented Oct 27, 2022

Hi @lucrp, can you post a screenshot of the overflow issue please? 🤔

Edit - I see it :-)

@Hlavtox
Copy link
Contributor

Hlavtox commented Oct 27, 2022

This is weird, I just built fresh develop branch of the theme and I have no issue like this. The gap between columns should not affect the row width at all.

Are you sure you didn't modify anything?

@lucrp
Copy link
Contributor Author

lucrp commented Oct 27, 2022

Hi @Hlavtox !
I tested again with a fresh develop branch install and I have the same overflow (both in Chrome and Firefox).
image

I found this problem with a little JS snippet that I use to find overflows like this, if you want to test it, just type this in your browser's console:

document.querySelectorAll('*').forEach(el => {
  if (el.offsetWidth > document.documentElement.offsetWidth) {
      console.log('Found the worst element ever: ', el);
  }
});

Adding col-12 to mobile devices in product__left class also fix the overflow problem, not need to change row g-5 class.
@lucrp
Copy link
Contributor Author

lucrp commented Oct 27, 2022

I found another way of solving that. If we add a col-12 in product__left and col in product__col overflow is fixed. Wdyt @Hlavtox ?

@lucrp
Copy link
Contributor Author

lucrp commented Oct 27, 2022

Sorry, not really.
image

I only solved it by changing g-5 to g-lg-5.

Only way I found to fix overflow in this page
@NeOMakinG
Copy link
Contributor

@lucrp we did investigate it, the col fix isn't the right one at all

The parent container has an inferior gutter than the row of these columns, I guess the right fix is probably about reducing the gutters on mobile or both desktop and mobile, we should really take care about the fact that he gutter helper is fine with the parent container

@lucrp
Copy link
Contributor Author

lucrp commented Oct 28, 2022

So, if I use p-4 or lower, it fixes the overflow. Am I right ? Otherwise, I did not understand how to change that. If you can explain me @Hlavtox, I'll appreciate it. Thanks in advance. 😄

@NeOMakinG
Copy link
Contributor

So, if I use p-4 or lower, it fixes the overflow. Am I right ? Otherwise, I did not understand how to change that. If you can explain me @Hlavtox, I'll appreciate it. Thanks in advance. 😄

g-4 g-xl-5 should fix it properly :)

@NeOMakinG
Copy link
Contributor

@Hlavtox feel free to merge if you feel it's good, should do the job!

@atomiix atomiix merged commit c6ee10c into PrestaShop:develop Dec 6, 2022
@atomiix
Copy link
Contributor

atomiix commented Dec 6, 2022

Thanks @lucrp!

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

Successfully merging this pull request may close these issues.

Product page overflow in mobile view
4 participants