Skip to content

Remove remaining swup:page:view listeners #2070

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 26, 2025
Merged

Remove remaining swup:page:view listeners #2070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions assets/js/sidebar/sidebar-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { el, getCurrentPageSidebarType, qs, qsAll } from '../helpers'
import { getSidebarNodes } from '../globals'

// Sidebar list is only rendered when needed.
// Mobile users may never see the sidebar.
let init = false
export function initialize () {
if (init) return
Expand Down Expand Up @@ -88,15 +90,15 @@ export function initialize () {
}))
})

window.addEventListener('hashchange', markCurrentHashInSidebar)

// We listen to swup:page:view event because we need to trigger
// markCurrentHashInSidebar() before scollNodeListToCurrentCategory.
window.addEventListener('swup:page:view', markCurrentHashInSidebar)
// Update new sidebar list with current hash.
markCurrentHashInSidebar()

// Triggers layout, defer.
requestAnimationFrame(scrollNodeListToCurrentCategory)

// Keep updated with future changes.
window.addEventListener('hashchange', markCurrentHashInSidebar)
window.addEventListener('exdoc:loaded', markCurrentHashInSidebar)
Copy link
Member

@josevalim josevalim Jan 25, 2025

Choose a reason for hiding this comment

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

Doesn't this mean markCurrentHashInSidebar will be invoked two times on boot? One here and another on DOMContentLoaded? Or is that fine?

Copy link
Contributor Author

@liamcmitchell liamcmitchell Jan 25, 2025

Choose a reason for hiding this comment

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

Calling multiple times is fine but I don't think it should happen.
On page load this would be called by the sidebar init fn which is now triggered by ex doc:loaded, meaning the event has already fired at this point and won't fire again until SWUP nav.
That's why it broke when you tried it. Adding the listener here is too late for the first render.

Copy link
Member

@josevalim josevalim Jan 26, 2025

Choose a reason for hiding this comment

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

Got it! Thanks for clarifying and sorry for being a slowpoke!

}

/**
Expand Down
16 changes: 10 additions & 6 deletions assets/js/swup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@ import SwupA11yPlugin from '@swup/a11y-plugin'
import SwupProgressPlugin from '@swup/progress-plugin'
import { isEmbedded } from './globals'

window.addEventListener('DOMContentLoaded', () => {
// Emit exdoc:loaded each time content loads:
// - on initial page load (DOMContentLoaded)
// - on subsequent SWUP page loads (page:view)
const emitExdocLoaded = () => {
window.dispatchEvent(new Event('exdoc:loaded'))
})
}

window.addEventListener('DOMContentLoaded', emitExdocLoaded)

if (!isEmbedded && window.location.protocol !== 'file:') {
new Swup({
Expand All @@ -17,10 +22,9 @@ if (!isEmbedded && window.location.protocol !== 'file:') {
path === window.location.pathname + '.html'
},
linkSelector: 'a[href]:not([href^="/"]):not([href^="http"])',
hooks: {
'page:view': emitExdocLoaded
},
plugins: [new SwupA11yPlugin(), new SwupProgressPlugin({delay: 500})]
})

window.addEventListener('swup:page:view', () => {
window.dispatchEvent(new Event('exdoc:loaded'))
})
}