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

Added footer in 404 page #212

Merged
merged 2 commits into from
Dec 2, 2021
Merged

Conversation

subhangi2731
Copy link
Contributor

Closes #211

Added footer in 404-error page.

@darknao
Copy link
Collaborator

darknao commented Nov 3, 2021

Mmhh.. I'm not sure, something looks off with the footer taking the full width, and not being completely at the bottom when using a large enough window.

@subhangi2731
Copy link
Contributor Author

@darknao Can you share a screenshot of the footer being off the bottom? In my case, the 404 page footer is still at the very bottom of the page in all window sizes.
Regarding the width, as you may notice, the 404 page's content takes up the whole page in comparison to meeting-details page where a box within the page shows the details and contains the footer.

@darknao
Copy link
Collaborator

darknao commented Nov 3, 2021

image

@subhangi2731
Copy link
Contributor Author

In my case, the footer looks as usual
Screenshot from 2021-11-04 08-23-08

Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Same for me, I guess.

image

@subhangi2731, could you check if there's an inbuilt footer component packed in Bootstrap 5?

@subhangi2731
Copy link
Contributor Author

@t0xic0der @darknao With the help of the in-built bootstrap I can add a footer in all the pages but this is a part of this issue #170. So if you can assign me that issue both will be covered under the same PR.
Screenshot from 2021-11-04 08-51-38

@subhangi2731
Copy link
Contributor Author

@darknao I have updated the changes please review

@subhangi2731
Copy link
Contributor Author

@t0xic0der @darknao please review my updated PR

@darknao
Copy link
Collaborator

darknao commented Nov 23, 2021

This is still not working.
Check this bootstrap example for reference : https://getbootstrap.com/docs/5.0/examples/sticky-footer-navbar/
Note that you need to use flexbox to make it work like that.

But there is several way to solve this issue.
If you look at others pages, you'll notice the footer is actually a card footer.
Why not just use this same logic here, and move everything in a card, like in the main page?

@subhangi2731
Copy link
Contributor Author

@darknao okay I will do this and update the PR

Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Why the long padding at the top?

image

@subhangi2731
Copy link
Contributor Author

@t0xic0der sorry will correct the padding other than this is everything okay?

@gridhead
Copy link
Member

@subhangi2731, I'd suggest that you correct the padding first and then maybe, I'll give it another round of review to see if there's anything else that needs changing.

@subhangi2731
Copy link
Contributor Author

subhangi2731 commented Nov 30, 2021

@t0xic0der sure I will fix this and also the card component so that the footer gets aligned downwards in its position

Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

LGTM.

Merging.

@gridhead gridhead self-requested a review December 1, 2021 03:36
Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Please ignore #212 (review). Changes are still needed.

@subhangi2731 subhangi2731 force-pushed the 404-page-footer branch 2 times, most recently from 7f352c9 to 7798c59 Compare December 1, 2021 14:40
@subhangi2731
Copy link
Contributor Author

@t0xic0der @darknao I corrected the long padding, kept all the contents in the card component, and then also brought the footer in correct alignment and now it's totally down touching the end of the page. Please check and let me know

@subhangi2731
Copy link
Contributor Author

Screenshot from 2021-12-01 20-23-03

@subhangi2731
Copy link
Contributor Author

subhangi2731 commented Dec 1, 2021

@darknao sure

@subhangi2731
Copy link
Contributor Author

@darknao please check and let me know

Copy link
Collaborator

@darknao darknao left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Made one small change to bring the card segment to the center of the page.

@gridhead gridhead merged commit 24d3764 into fedora-infra:main Dec 2, 2021
@subhangi2731
Copy link
Contributor Author

subhangi2731 commented Dec 2, 2021

@t0xic0der great thanks

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.

Add footer to error page
3 participants