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

Style back-to-top button to make sidebar more comfortable #1367

Merged
merged 3 commits into from
Feb 14, 2020

Conversation

backrunner
Copy link
Contributor

Add margin-bottom: 10px to the root div, to make this part more comfortable.
Just a style patch.

PR Checklist

  • The commit message follows guidelines for NexT.
  • Tests for the changes was maked (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs in NexT website have been added / updated (for features).

PR Type

  • 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?

Issue resolved: N/A

What is the new behavior?

  • Screenshots with this changes:
    Before adding an extra margin:
    image
    Seems like the bottom of the links-of-blogroll is too narrow, may cause a weird feeling.

After applied this change:
image

Does this PR introduce a breaking change?

  • Yes.
  • No.

@welcome
Copy link

welcome bot commented Feb 5, 2020

Thanks so much for opening your first PR here!

@claassistantio
Copy link

claassistantio commented Feb 5, 2020

CLA assistant check
All committers have signed the CLA.

@stevenjoezhang
Copy link
Contributor

I think the cause of the problem is the back-to-top button. Without back-to-top enabled, there would be too much white space.

截屏2020-02-05下午2 42 06

截屏2020-02-05下午2 42 10

You can try to style the back-to-top button to make it look better. However, there are some js files that need to be changed accordingly:

// Margin of sidebar b2t: 8px -10px -20px, brings a different of 12px.

@backrunner
Copy link
Contributor Author

backrunner commented Feb 5, 2020

I think the cause of the problem is the back-to-top button. Without back-to-top enabled, there would be too much white space.

How about adding an extra margin to .back-to-top-on?
Here's my codes (inject):

.back-to-top {
  margin: -4px -10px -18px;
  // or 0px -10px -18px
}
.back-to-top-on {
  margin-top: 16px;
}

In the original style, the back2top button looks like overflow, so I set the bottom margin to -18px.
image

After applying these styles:

With back2top button:
image

Without back2top button:
image

With friend links:
image
image

I don't know if the dynamic height of sidebar could cause some problem.
I changed the lines you mentioned like this:

// Margin of sidebar b2t: -4px -10px -18px, brings a different of 22px.
if (CONFIG.scheme === 'Pisces' || CONFIG.scheme === 'Gemini') sidebarSchemePadding += (sidebarOffset * 2) - 22;

@backrunner backrunner changed the title Make links-of-blogroll more comfortable Style back-to-top button to make sidebar more comfortable Feb 5, 2020
@WaldronWhy
Copy link

WaldronWhy commented Feb 5, 2020 via email

@@ -15,6 +15,7 @@
&.back-to-top-on {
cursor: pointer;
opacity: $b2t-opacity;
margin-top: 16px;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest reducing the margin here from 16px to 8px. This line of code affects all four schemes, not just Pisces and Gemini. 16px is too big for the other two schemes (Muse and Mist).

Suggested change
margin-top: 16px;
margin-top: 8px;

And it looks like this after modification

截屏2020-02-08下午3 19 02

Do you have a better suggestion?

Copy link
Contributor Author

@backrunner backrunner Feb 8, 2020

Choose a reason for hiding this comment

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

If it is acceptable, I think the following line could be better:

margin-top: ($scheme == 'Muse' || $scheme == 'Mist') ? -4px : 16px;

IMO, the best way is only apply this patch to Pisces and Gemini, put my patch into the scheme file (/source/css/_schemes/Pisces/_sidebar.styl), but that could make project more difficult to maintain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Put codes into scheme file can avoid the position of back-to-top button to be too high.
Cause the final margin-top in .back-to-button changed from 8px to -4px, so the button will be higher than before when Muse or Mist is enabled (in the post pages), but the original margin-top seems more suitable.
So what's your opinion about this?

@stevenjoezhang stevenjoezhang added this to the 7.7.2 milestone Feb 14, 2020
@stevenjoezhang stevenjoezhang merged commit a2bca35 into theme-next:master Feb 14, 2020
@welcome
Copy link

welcome bot commented Feb 14, 2020

Congrats on merging your first pull request here! 🎉 How awesome!

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.

4 participants