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

Align footer-links in the center of the page #3599

Merged
merged 1 commit into from Nov 14, 2018

Conversation

@nop33
Copy link
Member

commented Oct 17, 2018

I noticed that the footer links are clearly not in the center of the page and it seems like it was intended to be. This PR fixes this issue. To demonstrate:

Before

screen shot 2018-10-17 at 23 21 36

After

screen shot 2018-10-17 at 23 22 34

Also, is there a reason why the right padding of the footer is 5px but the left is 10px in indico/web/client/styles/partials/_footer.scss?

@@ -4,7 +4,7 @@
<a href="https://getindico.io">Indico</a>
{%- endset -%}
<div class="flexrow f-j-space-between">
<div class="flexrow f-a-center">
<div class="flexrow f-a-center f-self-stretch">

This comment has been minimized.

Copy link
@ThiefMaster

ThiefMaster Oct 19, 2018

Member

I'm curious, why do we need to stretch the outer elements for this? Shouldn't space-between on the container already spread them evenly?

This comment has been minimized.

Copy link
@nop33

nop33 Oct 20, 2018

Author Member

If all items have flex-grow set to 1, the remaining space in the container will be distributed equally to all children. In this case we have 3 children and we want the middle one to be at the exact center. So distributing the remaining space to the first and third child, we make sure that the 2nd one will always be in the middle.

This comment has been minimized.

Copy link
@nop33

nop33 Oct 20, 2018

Author Member

Even if the third element has no content ;)

@ThiefMaster ThiefMaster force-pushed the nop33:footer-links branch from 0009d53 to 7a8330b Nov 14, 2018

@ThiefMaster ThiefMaster force-pushed the nop33:footer-links branch from 7a8330b to a850b1a Nov 14, 2018

@ThiefMaster ThiefMaster merged commit a850b1a into indico:master Nov 14, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@nop33

This comment has been minimized.

Copy link
Member Author

commented Nov 14, 2018

yay, merged!

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Nov 26, 2018

Hey @nop33, I just noticed that this PR broke the footer logo text on the login page:

image

I know it's not october anymore, but would you be willing to fix that?

@nop33

This comment has been minimized.

Copy link
Member Author

commented Nov 26, 2018

Sure, I can have a look!

@nop33

This comment has been minimized.

Copy link
Member Author

commented Nov 26, 2018

Pulled from master.

Running ./bin/maintenance/build-assets.py indico --dev --watch I get these errors: https://gist.github.com/nop33/1616707b11fd9bb4e8b62d5d5a33be10

Running npm install I get these errors:

› npm install
npm ERR! code ENOLOCAL
npm ERR! Could not install from "resolver" as it does not contain a package.json file.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/iliastrichopoulos/.npm/_logs/2018-11-26T18_27_14_158Z-debug.log
@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Nov 26, 2018

rm -rf node_modules is the easiest option; npm doesn't like broken symlinks in there

@nop33

This comment has been minimized.

Copy link
Member Author

commented Nov 26, 2018

same result

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Nov 26, 2018

try deleting package-lock.json as well

@nop33

This comment has been minimized.

Copy link
Member Author

commented Nov 26, 2018

I did it right before I saw your comment. Worked, thx!

@nop33 nop33 referenced this pull request Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.