Skip to content

fix: Prevent initial unnecessary IntersectionObserver callback execution - #2523

Merged
sy-records merged 3 commits into
docsifyjs:developfrom
sy-records:fix/sidebar
Jan 6, 2025
Merged

fix: Prevent initial unnecessary IntersectionObserver callback execution#2523
sy-records merged 3 commits into
docsifyjs:developfrom
sy-records:fix/sidebar

Conversation

@sy-records

@sy-recordssy-records commented Jan 3, 2025

Copy link
Copy Markdown
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

@vercel

vercelBot commented Jan 3, 2025•
edited
Loading

Copy link
Copy Markdown

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

NameStatusPreviewCommentsUpdated (UTC)
docsify-preview✅ Ready (Inspect)Visit Preview💬 Add feedbackJan 5, 2025 11:21am

paulhibbitts
paulhibbitts previously approved these changes Jan 3, 2025
View reviewed changes

@paulhibbittspaulhibbitts left a comment

Copy link
Copy Markdown
Member

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 teamJanuary 4, 2025 02:34
jhildenbiddle
jhildenbiddle requested changes Jan 4, 2025
View reviewed changes

@jhildenbiddlejhildenbiddle left a comment

Copy link
Copy Markdown
Member

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.

Comment threadsrc/core/event/index.js Outdated
Comment threadsrc/core/event/index.js
Comment threadsrc/core/event/index.js
Koooooo-7
Koooooo-7 reviewed Jan 5, 2025
View reviewed changes

@Koooooo-7Koooooo-7 left a comment

Copy link
Copy Markdown
Member

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.

paulhibbitts
paulhibbitts approved these changes Jan 5, 2025
View reviewed changes

@paulhibbittspaulhibbitts left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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:developJan 6, 2025
@sy-records
sy-records deleted the fix/sidebar branch January 6, 2025 03:43
@sy-recordssy-records mentioned this pull request May 5, 2025
Merged
@sy-recordssy-records mentioned this pull request Jul 17, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Koooooo-7Koooooo-7Koooooo-7 left review comments
@jhildenbiddlejhildenbiddlejhildenbiddle approved these changes
@paulhibbittspaulhibbittspaulhibbitts approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

4 participants

@sy-records@jhildenbiddle@paulhibbitts@Koooooo-7