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: initialize progress value with init scroll #288

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alvarosabu
Copy link
Member

This PR fixes the issue with the progress value being always 0 even if the page is refreshed after scrolling to a certain position. (Astro)

Copy link

stackblitz bot commented Nov 21, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

netlify bot commented Nov 21, 2023

Deploy Preview for cientos-tresjs ready!

Name Link
🔨 Latest commit 9fc8f4c
🔍 Latest deploy log https://app.netlify.com/sites/cientos-tresjs/deploys/655d0bcc0330ce0008a261e2
😎 Deploy Preview https://deploy-preview-288--cientos-tresjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -112,21 +111,21 @@ watch(
},
)

watch(windowY, (value) => {
if (!isScrolling.value && !props.htmlScroll) return
function updateScroll(value: number) {
progressScroll.value = (value / height.value / (scrollNodeY.value / height.value - 1))
Copy link
Member

Choose a reason for hiding this comment

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

How is this working for horizontal axis??

Copy link
Member Author

Choose a reason for hiding this comment

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

True

@alvarosabu alvarosabu added the v4 label Nov 28, 2023
Copy link
Contributor

@andretchen0 andretchen0 left a comment

Choose a reason for hiding this comment

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

It works! This is a welcome improvement.

I'm approving since the fix works, but I noticed this related bug.

Bug: linking to named anchors

Reproduction

Example markup

<a href="#two">Jump to section 2</a>
<section><h1>Section 1</h1></section>
<section><h2 id="#two">Section 2</h2></section>
  1. In the text editor, insert the example markup into the playground/src/pages/controls/ScrollControlsDemo.vue and save.
  2. On the command line, start the playground with pnpm run playground if not already running.
  3. In the browser, open a new, blank tab.
  4. In the browser, paste http://localhost:5173/controls/scroll-controls#two in the address bar and hit Enter.

Expected behavior

The page should scroll to the second header.

Behavior

The page loads and remains at y-offset 0.

Once the page is loaded, clicking on "Jump to section 2" works as expected.

Tested in Chrome, FF, Safari on Mac.


Would you like this to be opened as a separate issue or worked on here?

@alvarosabu
Copy link
Member Author

Hi @andretchen0 thanks for the help, yes let's open it on a separate issue to focus on anchors

@JaimeTorrealba JaimeTorrealba added bug Something isn't working waiting for author p3-minor-bug An edge case that only affects very specific usage (priority) labels Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p3-minor-bug An edge case that only affects very specific usage (priority) v4 waiting for author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants