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

FIX: Better way to adjust footer position in Pisces. #1807

Merged
merged 2 commits into from
Aug 12, 2017
Merged

FIX: Better way to adjust footer position in Pisces. #1807

merged 2 commits into from
Aug 12, 2017

Conversation

AlynxZhou
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes have been added (for bug fixes / features).
  • Docs have been added / updated (for bug fixes / features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Like this image, if you add some custom sentences or just enable busuanzi like me, you'll find that footer height didn't increase as you want, and that would cause a bug when the inner HTML is too much, it will going into main part.

next_bug

I fixed this problem, very simply, I read the css code and find NexT use a strange way to place the footer, that is let main use absolute position in the bottom and a big padding-bottom, then let the footer show above this padding-bottom, but the padding-bottom won't change automatically. I just removed those css, added a padding-top to footer, then the footer will show after the main, and main's padding-bottom have nothing with footer, just the way every simple HTML page use.

Issue Number(s): N/A

What is the new behavior?

Already tested and worked in my blog, perfectly.

Does this PR introduce a breaking change?

  • Yes.
  • No.

@ivan-nginx
Copy link
Collaborator

@AlynxZhou maybe not need padding-top: 20px, how u think?

@AlynxZhou
Copy link
Contributor Author

@ivan-nginx Maybe. I just think that main needs keep a distence with footer, just because this looks good, you can remove this line. I just think the old way NexT use is not so flexible.

@ivan-nginx
Copy link
Collaborator

Yes, footer padding/margin have more space. In my blog i reduce it, but not else syncronize with master branch because there are a lot of changes in custom styles, but i do it soon.
@AlynxZhou ok, it was tested on all schemes or only on Pisces/Gemini?

@AlynxZhou
Copy link
Contributor Author

@ivan-nginx I only find Pisces, because I only find padding-bottom in _scheme/Pisces, other path don't have this, but I think need further test for other schemes because they have a similar structure.

@ivan-nginx ivan-nginx merged commit 9ec73d8 into iissnan:master Aug 12, 2017
@ivan-nginx ivan-nginx added the Bug label Aug 17, 2017
@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Aug 17, 2017

@AlynxZhou there is some bug in Mist theme (#1818). Need to fix it, i think.

@AlynxZhou
Copy link
Contributor Author

@ivan-nginx I think that's because his page is too short (shorter than the browser window), however if set its position to absolute back, the padding bottom of main need to set again, because it won't change automatically. I cannot think a better way to solve both of them...

@AlynxZhou
Copy link
Contributor Author

@ivan-nginx I find the way in Gemini to adjust the position of footer works fine and add it into Pisces, then solved this problem. Maybe I should open another PR?

@ivan-nginx
Copy link
Collaborator

Maybe I should open another PR?

@AlynxZhou Sounds good. Need to fix it anyway, because and on Pisces and on Gemini if footer not absolute, he's will always stick to top page and on categories/tags pages footer will show at the center page — this is not right. Footer must always be in bottom.

@AlynxZhou
Copy link
Contributor Author

@ivan-nginx I finally changed it back, just change some variables to let it seems better, and changed Gemini, now its footer also stay in bottom. But honestly I want to add some custom lines to footer and in this way I need to change the variables for myself, there seems no easier way to set main's padding-bottom dynamically. Anyway, that's my own problem, I already fixed this bug, I will open another PR.

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Aug 23, 2017

I want to add some custom lines to footer and in this way I need to change the variables for myself

Why u not want to add something like _custom file to footer? Anything may be added with option in config in false by default, like Hexo and Next copyright info (github pages, for example).

there seems no easier way to set main's padding-bottom dynamically.

Why? U read about flex style? There is no impossible things, if u want to change something truly.


Show me, where u add your custom labels?

@ivan-nginx ivan-nginx added this to the v5.1.3 milestone Aug 23, 2017
@AlynxZhou
Copy link
Contributor Author

AlynxZhou commented Aug 24, 2017

Anything may be added with option in config in false by default, like Hexo and Next copyright info (github pages, for example).

@ivan-nginx I don't mean an option, I mean that I add something to my own site, then I need to change something, but not every one needs those things, so I didn't add them to the theme.

Why? U read about flex style? There is no impossible things, if u want to change something truly.

Now footer has a fixed height of 140px, this can contain 3 lines (copyright, theme info and busuanzi counter), but if I add to custom lines to footer such as "Hosted by GitHub Pages", 140px is too small that the word will expand to other part, then I need to custom the number. I really want a easy way to adjust the number due to total lines height, not by user, but I know little about CSS.
Except this, the new PR fixes bugs, now Gemini and Pisces' footer are in the bottom. I'll read flex soon.

@ivan-nginx
Copy link
Collaborator

@AlynxZhou ok, can u give me «Hosted by GitHub Pages» code? Go write comments to new #1824 pull.

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.

2 participants