-
Notifications
You must be signed in to change notification settings - Fork 26k
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
Scrollspy-style updating of hash and TOC highlighting #2020
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work yet again. Everything works as expected for me.
Only suggestion I have is styling of the .active
state.
There's a Sass mixin I use to help with doing light/dark or dark/light color combinations. Instead of hardcoding the active-color
variable as you have, you can use this function. This will allow more flexibility for the various color schemes the theme supports.
I've added notes to _variables.scss
and _navigation.scss
with suggested Sass changes to make this work.
Examples:
|
||
// Scrollspy marks toc items as .active when they are in focus | ||
.active { | ||
background: $active-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch to:
.active {
@include yiq-contrasted($active-color);
}
@@ -61,6 +61,7 @@ $muted-text-color: mix(#fff, $text-color, 35%) !default; | |||
$border-color: $lighter-gray !default; | |||
$form-background-color: $lighter-gray !default; | |||
$footer-background-color: $lighter-gray !default; | |||
$active-color: mix($light-gray, $lighter-gray, 50%) !default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch to the following and move after $primary-color
line to avoid Sass error.
$active-color: mix(#fff, $primary-color, 80%) !default;
Fixed, thanks! Sorry, I forgot about themes. I guess another question is whether there should be an option to turn the auto-hash-changing functionality off. But hopefully (most?) everyone wants it. |
I'm cool with leaving it on by default and not adding some sort of config. The theme is already a mess of configurations, don't need the added complexity 😆 I could see someone potentially complaining about the size of |
Hey, sorry to comment on this old issue, but is this working ? Automatic updating of browser location's hash fragment according to what's on display (while the user scrolls, but without interfering with fragment while smooth scrolling / when clicking on a link). I don't see my browser's location being updated ( like https://coffeescript.org/ ). |
It's been a while, but I believe Minimal Mistakes switched to using Gumshoe instead of my custom code, and Gumshoe sadly doesn't support automatic hash fragment. If @mmistakes is supportive, I could try to add some code back that does just the hash fragment part, perhaps enabled by an option? |
@edemaine thanks for replying! That 'd be great. Do you have this already on your website? Could you point me to that code so that I could add this, in case @mmistakes doesn't want to add this back ? |
I haven't tested yet, but something like this should work: document.addEventListener("gumshoeActivate", function (event) {
const target = event.target;
const targetLink = target.querySelector(":scope > a[href]");
if (!targetLink)
// Precautionary check, so that we don't fall into unexpected errors
return;
const hash = targetLink.getAttribute("href");
history.replaceState({}, "", hash);
}); Gumshoe's custom events are particularly useful here. |
@iBug legend! Just gave it a try and works like a charm. Many thanks! |
This is an enhancement or feature.
Summary
This PR adds two features:
To avoid slowdown while scrolling, I added a simple throttling library, and replaced existing code where you were throttling inefficiently with
setInterval
. This should improve idle performance while resizing identically. (I wasn't totally sure whether to usethrottle
ordebounce
, butthrottle
seems like a fine choice.)Context
Follow-up to #2019.
For testing the second feature, it is useful to have a test with sticky TOC, but I didn't add one. Let me know if you'd like a change or copy of
layout-table-of-contents-post
.