-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 scroll-spy behaviour (fixes #1289) #1300
Conversation
I'm fine with these changes in foc structure though it seems kind of weird Also can the .each loop be removed on l135 and be just a .addClass? On Friday, 7 October 2016, Hubert Argasinski [email protected]
|
Fixed, thanks for catching that! Should I squash the commits or is this good to merge? The main reason I'm changing the template so much after the docs are generated is because it's a lot quicker to support the |
Works for me in current state (though I haven't seen the actual behaviour On Sunday, 9 October 2016, Hubert Argasinski [email protected]
|
959deb3
to
5c0ac6e
Compare
Thank you! I just had to fix the searching as it was relying on the old ids. Everything appears to be working now. |
@hargasinski want to tag a release? |
@megawac, sure! Just to confirm, that'll mean updating the changelog, building the dist files and updating the version to I just want to confirm so I don't mess anything up 😩. Thanks! |
Workflow is updating the changelog and then running |
I should've looked at the |
@megawac I tagged the release. Everything went smoothly updating Github, except for one small linting error I fixed. However, |
done, had to release a 2.1.1 though On Wed, Oct 12, 2016 at 2:38 PM, Hubert Argasinski <[email protected]
|
Thanks! |
Good work, I like the enhancement! One thing to think about for the future is how to simplify the docs generation code. We are fighting the template a lot. |
I just wanted a second opinion on this PR because I had to change the ids of the method sections in the body for scroll-spy to work. I just had to remove the
.
in front the ids (.eachSeries
vseachSeries
now). This means that potentially old links to methods in the docs will just point to the top of the old page. I added some js that should minimise this, it replaces the hash in the link with the hash with the.
removed. It should direct users to the correct method.I think the ids are a result of the way minami generates them. Its something I should've changed initially when creating the docs. Sorry about that.
Also, to get scroll-spy to work, I had to rearrange some of the HTML/CSS, but from what I can see, everything looks the same as before. I have a live demo on my fork: http://hargasinski.github.io/async.
Thanks in advance for the feedback.
/cc @megawac @aearly @kalmanh