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

Fixing - Responsive Layout on footer #16981

Closed
wants to merge 2 commits into from
Closed

Fixing - Responsive Layout on footer #16981

wants to merge 2 commits into from

Conversation

rodrigoobiassi
Copy link
Member

I was showing the platform to a prospective client and noticed this problem. Broken responsive layout.

1 - test

2 - test2

@magento-engcom-team
Copy link
Contributor

Hi @rodrigoobiassi. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jul 20, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

@rodrigoobiassi thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@gwharton
Copy link
Contributor

Please check if PR #17006 fixes this for you and close as necessary.

@sidolov
Copy link
Contributor

sidolov commented Jul 29, 2018

@rodrigoobiassi , please, check comment above and take a look at merge conflict.
Thank you!

@rodrigoobiassi
Copy link
Member Author

Hi @sidolov, I just made the requested change. Thanks.

@gwharton
Copy link
Contributor

gwharton commented Jul 30, 2018

@rodrigoobiassi Are you sure this PR is still required. PR #17006 made changes to the responsive layout for mobile devices, namely it made the mobile screens behave the same way as the desktop screens with respect to the footers. i.e that if the page/content is shorter than the screen, then the footer snaps to the bottom whereas if the page/content is longer than the screen, then the footer is pushed to the bottom of the content, and not the bottom of the screen.

It looks like this PR is in conflict with this resolution, i.e that it will cause the footer to always snap to the bottom of the screen regardless of the page/content size. This is not consistent with the desktop layout.

@sidolov
Copy link
Contributor

sidolov commented Aug 7, 2018

Hi @rodrigoobiassi , please, read the comment from @gwharton , maybe the issue resolved and we can close this pr?

@sidolov
Copy link
Contributor

sidolov commented Aug 21, 2018

Hi @rodrigoobiassi , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

@sidolov sidolov closed this Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants