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: resolve footer overflow issue on tablet screens causing horizontal scroll #2339

Draft
wants to merge 1 commit into
base: testing
Choose a base branch
from

Conversation

habdelrahmane
Copy link

@habdelrahmane habdelrahmane commented Mar 7, 2025

Summary

This PR resolves an issue where the footer was overflowing horizontally on tablet view between 768px and 1230px, due to a recently changed CSS rule in the pageContainer mixin.
The overflow caused poor vertical scrolling performance, impacting the reading experience.

The issue is fixed by unsetting the width in the .flowItem class, ensuring proper layout without altering the utility.scss file.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test Plan

  • Verified that the horizontal overflow issue is resolved on an iPad and in browser developer tools for tablet view between 768px and 1230px.
  • Tested on different tablet devices and screen sizes to ensure consistent behavior.
  • Checked that no other layout issues were introduced, and confirmed that the footer now behaves correctly across tablet breakpoints.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New and existing unit tests pass locally with my changes

Screenshots or videos

Before After
Overflowing footer Corrected footer

Following PR#2308, the footer started to overflow on tablet view between 768px and 1230px. 

The root cause was a recently changed CSS rule in the `pageContainer` mixin, which the `.flowItem` class includes. 

Fixed the overflow by unsetting the width in `.flowItem`, ensuring that the `utility.scss` file remains untouched.

Link to the recently changed CSS rule in `_utility.scss` : https://github.com/quran/quran.com-frontend-next/blame/b670a6dd7909548db09eeda31f7ed6b5a29e1133/src/styles/_utility.scss#L849
Copy link

vercel bot commented Mar 7, 2025

@habdelrahmane is attempting to deploy a commit to the Quran Team on Vercel.

A member of the Team first needs to authorize it.

@habdelrahmane habdelrahmane marked this pull request as draft March 10, 2025 11:11
@habdelrahmane
Copy link
Author

I’ve temporarily drafted this PR as it only fixes the footer overflow issue while reading a Surah on tablet, thus greatly improving the reading experience by not having a horizontal scroll anymore.

However, the index page still has an overflow issue, because the index .flowItem also inherit the width CSS rule from pageContainer.

I didn’t alter the pageContainer mixin in _utility.scss in my initial PR.

I’m considering updating the PR by simply reverting the width to 80% in the pageContainer mixin on tablet, like it was before #2308.

@osamasayed may I ask you your thought about this?
May Our Lord bless you.

@habdelrahmane
Copy link
Author

@osamasayed @AhmedCodeGuy If you have a moment, could you please take a look at the quoted post please?
I’m happy and proud to provide this easy fix, with God’s will.
May Our Lord bless you, thanks!

I’ve temporarily drafted this PR as it only fixes the footer overflow issue while reading a Surah on tablet, thus greatly improving the reading experience by not having a horizontal scroll anymore.

However, the index page still has an overflow issue, because the index .flowItem also inherit the width CSS rule from pageContainer.

I didn’t alter the pageContainer mixin in _utility.scss in my initial PR.

I’m considering updating the PR by simply reverting the width to 80% in the pageContainer mixin on tablet, like it was before #2308.

@osamasayed may I ask you your thought about this? May Our Lord bless you.

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.

1 participant