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

Autoscroll after the whole page is loaded #364

Merged
merged 1 commit into from
Jul 24, 2018

Conversation

nusjzx
Copy link
Contributor

@nusjzx nusjzx commented Jul 24, 2018

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [ ] Documentation update
• [X] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [ ] Other, please explain:

Fixes #310

What is the rationale for this request?
when jumping to specific headings in other pages or subsites, content is not loaded before the scrolling.
Which makes it scrolling to the wrong place.

What changes did you make? (Give an overview)
scrolling after the page is loaded(use Vuejs mounted hook)

Testing instructions:

  1. markbind serve docs
  2. test the advanced tips and tricks link is scrolling to the correct place

@nusjzx nusjzx force-pushed the 310-autoscroll-after-page-loaded branch 2 times, most recently from cf6ca8d to 85c9ba0 Compare July 24, 2018 03:13
@nusjzx nusjzx requested a review from yamgent July 24, 2018 03:19
Copy link
Member

@yamgent yamgent left a comment

Choose a reason for hiding this comment

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

Just some minor nits:

@@ -2,6 +2,12 @@

Vue.use(VueStrap);

function scrollToHeading() {
Copy link
Member

Choose a reason for hiding this comment

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

You need to call this method in setup() as well.

I am not sure whether setup() is relevant anymore (siteData.json is always generated nowadays afaik?), but for consistency sake you should still add this call.

Copy link
Member

Choose a reason for hiding this comment

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

Also, a better name for the method can be scrollToUrlAnchorHeading().

@nusjzx nusjzx force-pushed the 310-autoscroll-after-page-loaded branch from 24cca0c to 1d1edc0 Compare July 24, 2018 03:45
@yamgent yamgent added this to the v1.9.2 milestone Jul 24, 2018
@yamgent yamgent merged commit 531a1bf into MarkBind:master Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase success rate of auto-scroll-to-target-subheading of search results
2 participants