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(footer): Adjust bottom margin based on scroll position to preserve safe area 2063 #2071

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zahidhussain998
Copy link

@zahidhussain998 zahidhussain998 commented Jun 24, 2024

This commit fixes the issue of incorrect bottom margin for the footer breadcrumbs during scrolling (#2063). The Footer component now dynamically adjusts the bottom margin based on the user's scroll position. A new state variable isScrolledToBottom was introduced to track the scroll position, and a useEffect hook handles scroll events to update this state. The bottom margin is conditionally applied to ensure it's only present when needed

…e safe area

This commit fixes the issue of incorrect bottom margin for the footer breadcrumbs during scrolling. The Footer component now dynamically adjusts the bottom margin based on the user's scroll position. A new state variable isScrolledToBottom was introduced to track the scroll position, and a useEffect hook handles scroll events to update this state. The bottom margin is conditionally applied to ensure it's only present when needed
@zahidhussain998 zahidhussain998 changed the title fix(footer): Adjust bottom margin based on scroll position to preserv… fix(footer): Adjust bottom margin based on scroll position to preserve safe area 2063 Jun 24, 2024
Copy link
Contributor

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your contribution.

There are a couple issues with the proposed solution.

  1. The issues specifies "If the safe area is partially obscured, the bottom margin should adjust accordingly." That could be said more clearly, but basically it means that the bottom margin needs to be adjusted based on the distance from the bottom of the page. So isScrolledToBottom: number should probably be distanceFromScrollBottom: number instead.
  2. The .nav-bottom .nav-container already uses --safe-area-inset-bottom. The solution should develop an integrated solution applied to the same element, rather than a separate solution applied to the footer.

…sted the bottom margin dynamically.

updated state management changed from isScrolledToBottom to distanceFromScrollBottom and adjusted the bottom margin dynamically.
@raineorshine
Copy link
Contributor

Checking in. A solution needs to be developed that takes into account the existing safe area logic. Are you still interested in this, or should I close?

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.

2 participants