Skip to content

Conversation

sy-records
Copy link
Member

Summary

  • Added a flag (isInitialLoad) to skip the first callback execution of the IntersectionObserver on page load.

Related issue, if any:

Fix #2504
Fix #2509

What kind of change does this PR introduce?

Bugfix

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

Yes
No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge

Copy link

vercel bot commented Jan 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 5, 2025 11:21am

paulhibbitts
paulhibbitts previously approved these changes Jan 3, 2025
Copy link
Collaborator

@paulhibbitts paulhibbitts left a comment

Choose a reason for hiding this comment

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

Thanks @sy-records , created Sandbox and tested (see issue comments) - all looks good to me!

@sy-records sy-records requested a review from a team January 4, 2025 02:34
Copy link
Member

@jhildenbiddle jhildenbiddle left a comment

Choose a reason for hiding this comment

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

Looks good, @sy-records. A few minor nitpicks.

Copy link
Member

@Koooooo-7 Koooooo-7 left a comment

Choose a reason for hiding this comment

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

Shall we promote the isInitialLoad to the Object level as a field?
Then, we could reuse and keep consistence when any logic relies on the same isInitialLoad lifecycle flag in future.

Copy link
Collaborator

@paulhibbitts paulhibbitts left a comment

Choose a reason for hiding this comment

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

Thanks @sy-records and @jhildenbiddle , I've retested in the sandbox (with new Preview build) and things look good to me!

https://codesandbox.io/p/sandbox/docsify-v5-sidebar-fix-zwdm6p

@sy-records sy-records merged commit a73e07e into docsifyjs:develop Jan 6, 2025
9 checks passed
@sy-records sy-records deleted the fix/sidebar branch January 6, 2025 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants