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

Fixed an issue where the page content was overlapped by footer components #353

Merged

Conversation

fkloes
Copy link
Collaborator

@fkloes fkloes commented Oct 31, 2018

Description

On an iPhone X the PaymentBar and the AddToCartBar don't overlap the page content anymore when the user scrolls to the bottom of the page.

Type of change

  • Bug Fix 🐛 (non-breaking change which fixes an issue)
  • [ ] New Feature 🚀 (non-breaking change which adds functionality)
  • [ ] Breaking Change 💥 (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Polish 💅 (Just some cleanups)
  • [ ] Docs 📝 (Changes in the documentations)
  • [ ] Internal 🏠 Only relates to internal processes.

How to test it

  • put some products into the cart on an iPhone X and check if the PaymentBar overlaps the cart when you scroll to the bottom of the page.
  • goto a PDP and check if the AddToCartBar overlaps page content when you scroll to the bottom of the page.

@fkloes fkloes added the bug Something isn't working label Oct 31, 2018
@fkloes fkloes self-assigned this Oct 31, 2018
@coveralls
Copy link

coveralls commented Oct 31, 2018

Pull Request Test Coverage Report for Build 1384

  • 1 of 4 (25.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 46.844%

Changes Missing Coverage Covered Lines Changed/Added Lines %
themes/theme-ios11/pages/Product/components/AddToCartBar/style.js 0 1 0.0%
themes/theme-gmd/components/View/components/Content/style.js 1 3 33.33%
Totals Coverage Status
Change from base Build 1383: -0.02%
Covered Lines: 4849
Relevant Lines: 9788

💛 - Coveralls

reneeichhorn pushed a commit that referenced this pull request Oct 31, 2018
Copy link
Contributor

@richardgorman richardgorman left a comment

Choose a reason for hiding this comment

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

Please take into consideration the recent changes regarding the footer contents.

richardgorman and others added 4 commits November 1, 2018 10:43
…age content

- added ContentBelow View component logic to the AddToCartBar
- updated ContentBelow component to consider page insets
- fixed an GMD issue where the category content was scrollable horizontally when the FilterBar was visible
Copy link
Contributor

@richardgorman richardgorman left a comment

Choose a reason for hiding this comment

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

Please consider using the Footer portal to inject the bar. It should remove some complexity around this PaymentBar

@fkloes
Copy link
Collaborator Author

fkloes commented Nov 5, 2018

Putting the PaymentBar into the Footer portal would require some bigger refactoring of the component, since it's already putted into kind of a portal. Since the PaymentBar is also used within the GMD theme - other than the AddToCartBar and the TabBar - i'd also have to extend the Viewport with a footer section to keep the themes in sync.
Since this will cause some extra unplanned work i think this shouldn't be done within this ticket.

@@ -19,7 +19,7 @@ const content = (isFullscreen = false, keyboardHeight = 0, noScroll = false) =>
width: '100vw',
position: 'absolute',
top: 0,
paddingBottom: `calc(var(--tabbar-height) + ${keyboardHeight}px + var(--safe-area-inset-bottom))`,
paddingBottom: `${keyboardHeight}px`,
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for a template string here

@@ -6,30 +6,35 @@ import variables from 'Styles/variables';
* @param {boolean} isFullscreen Whether remove all offsets,
* so that it's really fullscreen (including the notch).
* @param {number} keyboardHeight The space that is taken by the keyboard.
* @param {boolean} noScroll Wheather the view should be scrollable or not.
Copy link
Contributor

Choose a reason for hiding this comment

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

Type: whether

@richardgorman richardgorman merged commit 79bbd3a into PWA-327-Exchange-Router Nov 5, 2018
@richardgorman richardgorman deleted the PWA-1157-payment-bar-overlaps-cart branch November 5, 2018 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants