Bugfix/footer

[Asu1996]

Fixes #5059 (<=== Add issue number here)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• PR is descriptively titled 📑 and links the original issue above 🔗
• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
• code is in uniquely-named feature branch and has no merge conflicts 📁
• screenshots/GIFs are attached 📎 in case of UI updation
• ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

[welcome]

Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help.
Dangerbot will test out your code and reply in a bit with some pointers and requests.
Also please refer here for installation help 💿
There may be some errors, but don't worry! We'll work through them with you! 👍🎉😄
It would be great if you can tell us your Twitter handle so we can thank you properly?

[gitpod-io]

[codecov]

Codecov Report

Merging #8537 into main will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #8537   +/-   ##
=======================================
  Coverage   81.91%   81.91%           
=======================================
  Files         101      101           
  Lines        5894     5896    +2     
=======================================
+ Hits         4828     4830    +2     
  Misses       1066     1066           

Impacted Files	Coverage Δ
app/mailers/comment_mailer.rb	100.00% <100.00%> (ø)
app/mailers/subscription_mailer.rb	94.33% <100.00%> (+0.22%)	⬆️

[codeclimate]

Code Climate has analyzed commit 9729e93 and detected 0 issues on this pull request.

View more on Code Climate.

[jywarren]

Hmm, I'm a little worried about modifying default Bootstrap styles here, and about modifying the HTML and body styles... Is there a more minimal way to do this, or is this the only way?

Would it work to just absolute position the footer? And, I guess, add corresponding amount of bottom-padding to the body so it doesn't overlap anything?

Thanks for working through this with us, just trying to be sure it works on a variety of screen sizes and is maintainable and Bootstrap compatible! Thanks a lot!

[jywarren]

What do you think, @avats-dev of my questions above? Thanks both of you!!!

[avats-dev]

As far as I know, you will need to modify body container along with footer div for achieving this. Setting it absolute has a drawback that if there is more content, even then footer will be stuck at bottom. So, it's best to attach it to page container instead of viewport.

You can see this for reference.

We can also use flex, but that will also require editing body div.

[Asu1996]

@avats-dev Sir, should i work on doing a better code?

[jywarren]

Hi, well, hmm -- in testing this in GitPod, i see some other layout issues resulting - like, here on tag pages: /tag/test -- there's extra white space at the top of the page:

(compare to https://publiclab.org/tag/outreachy)

However, other pages seem OK! Even in mobile-friendly page-widths:

However, I see as @avats-dev mentioned, this doesn't happen much because our footer is quite full of content now.

Balancing all of this, I guess I think we'd better leave the layout more standard and not make these changes, given that any customization will make our layouts slightly more brittle and harder to maintain. I want to apologize to @Asu1996 who put good work into this and solved the stated issue, and will make a new issue for @Asu1996 to make up for this, hopefully.

Thanks, everyone, for your help on this, and my apologies for the reversal. I appreciate your patience!

[jywarren]

I'm pulling one from #8545 - will post the issue here for @Asu1996 as soon as it's posted!

[jywarren]

Here you go @Asu1996 ! #8546