diff --git a/vue/src/components/AppNav.vue b/vue/src/components/AppNav.vue index d3f8e89..ab1dec3 100644 --- a/vue/src/components/AppNav.vue +++ b/vue/src/components/AppNav.vue @@ -1,96 +1,23 @@ - + RSS Reader ({{ unreadCount }}) cb()) - -function scrollTo(y) { - Object.defineProperty(window, 'scrollY', { value: y, configurable: true, writable: true }) - window.dispatchEvent(new Event('scroll')) -} describe('AppNav', () => { let router @@ -29,7 +23,6 @@ describe('AppNav', () => { localStorage.setItem('user-token', 'abc123') localStorage.setItem('user-id', '7') vi.clearAllMocks() - scrollTo(0) const { feeds, showMessage, message, showModal, viewMode, currentIndex, layout } = useFeeds() feeds.value = [] @@ -51,17 +44,8 @@ describe('AppNav', () => { await router.isReady() }) - // Safety net: if a test using fake timers fails before reaching its own - // vi.useRealTimers(), real timers must still be restored so it doesn't - // silently break the rAF-dependent scroll handling in later tests (fake - // timers by default also fake requestAnimationFrame). - // - // Also unmounts every AppNav mounted via mountNav() this test — without - // this, each test's `window.addEventListener('scroll', ...)` from AppNav's - // onMounted piles up across the whole file (nothing ever unmounts them - // otherwise), so a later scroll-correction test can have its - // consumeScrollCorrection() flag "stolen" by a stale listener from an - // earlier, unrelated test that happens to still be registered. + // Unmount every AppNav mounted via mountNav() after each test so mounted + // instances (and their router/menu listeners) don't pile up across the file. let mountedWrappers = [] function mountNav(options = { global: { plugins: [router] } }) { const wrapper = mount(AppNav, options) @@ -70,7 +54,6 @@ describe('AppNav', () => { } afterEach(() => { - vi.useRealTimers() for (const wrapper of mountedWrappers) { try { wrapper.unmount() @@ -246,151 +229,4 @@ describe('AppNav', () => { confirmSpy.mockRestore() }) - it('compacts the header once scrolled past the enter threshold', async () => { - const wrapper = mountNav() - - scrollTo(100) - await wrapper.vm.$nextTick() - - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - }) - - it('stays expanded for scroll offsets inside the hysteresis dead zone', async () => { - const wrapper = mountNav() - - scrollTo(50) - await wrapper.vm.$nextTick() - - expect(wrapper.find('.app-nav').classes()).not.toContain('app-nav--compact') - }) - - it('expands the header again once scrolled back near the top', async () => { - vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) - const wrapper = mountNav() - - scrollTo(100) - await wrapper.vm.$nextTick() - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - scrollTo(10) - await wrapper.vm.$nextTick() - // The expand transition is debounced (see the article-read scroll-dip - // test below), so it shouldn't flip immediately... - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - vi.advanceTimersByTime(150) - await wrapper.vm.$nextTick() - // ...but should once the debounce window elapses with scrollY still low. - expect(wrapper.find('.app-nav').classes()).not.toContain('app-nav--compact') - - vi.useRealTimers() - }) - - it('does not flash expanded on a brief scroll dip below the exit threshold', async () => { - // Regression test: useFeeds.js's handleIntersection corrects the scroll - // position with a window.scrollBy after marking an article read, which can - // transiently dip scrollY below COMPACT_EXIT even while the user keeps - // scrolling down. That one-off dip must not visibly re-expand the header. - vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) - const wrapper = mountNav() - - scrollTo(100) - await wrapper.vm.$nextTick() - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - scrollTo(10) // the corrective dip - await wrapper.vm.$nextTick() - scrollTo(80) // scrolling continues downward right after - await wrapper.vm.$nextTick() - - vi.advanceTimersByTime(200) - await wrapper.vm.$nextTick() - - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - vi.useRealTimers() - }) - - it('stays compact through a real article-read scroll correction, even after a sustained pause', async () => { - // Regression test for the reported glitch: unlike a brief flash, marking - // an article read (list view) can leave scrollY genuinely low for as - // long as the user pauses between scroll gestures — e.g. right when they - // finish one article and are about to start the next — because the list - // view removes read articles from the DOM, collapsing the remainder back - // toward the top of the page. A fixed debounce alone can't distinguish - // that from a real "scrolled back to the top". This drives the actual - // useFeeds.js handleIntersection correction (not a simulated dip) and - // confirms the header stays compact even once the debounce window has - // fully elapsed with no further scrolling. - vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) - const { feeds: feedsRef, handleIntersection, setInitialLoad } = useFeeds() - feedsRef.value = [{ id: 201, title: 'Only article' }] - setInitialLoad(true) - axios.put.mockResolvedValue({ status: 200 }) - - const wrapper = mountNav() - - scrollTo(100) - await wrapper.vm.$nextTick() - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - // jsdom's getBoundingClientRect().top defaults to 0 for any element, so a - // topbarHeight > 0 exercises the same "article now sits above the header" - // correction branch a real browser would take. - const observeDiv = document.createElement('div') - observeDiv.className = 'observe' - document.body.appendChild(observeDiv) - await handleIntersection([ - { isIntersecting: false, boundingClientRect: { y: -10 }, target: { id: '0' } }, - ], 60) - await flushPromises() - - // The real window.scrollBy correction would land scrollY low, like this. - scrollTo(10) - await wrapper.vm.$nextTick() - - // Long past the debounce window, with no further scrolling — simulating - // the user pausing right at the top of the next article. - vi.advanceTimersByTime(300) - await wrapper.vm.$nextTick() - - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - document.body.removeChild(observeDiv) - setInitialLoad(false) - vi.useRealTimers() - }) - - it('stays compact while scrolling within the hysteresis dead zone', async () => { - const wrapper = mountNav() - - scrollTo(100) - await wrapper.vm.$nextTick() - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - - scrollTo(50) - await wrapper.vm.$nextTick() - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - }) - - it('does not throw on rapid repeated scroll events', async () => { - const wrapper = mountNav() - - for (let y = 0; y <= 200; y += 5) { - scrollTo(y) - } - await wrapper.vm.$nextTick() - - expect(wrapper.find('.app-nav').classes()).toContain('app-nav--compact') - }) - - it('removes the scroll listener on unmount', async () => { - const removeSpy = vi.spyOn(window, 'removeEventListener') - const wrapper = mountNav() - - wrapper.unmount() - - expect(removeSpy).toHaveBeenCalledWith('scroll', expect.any(Function)) - removeSpy.mockRestore() - }) }) diff --git a/vue/src/composables/__tests__/useFeeds.spec.js b/vue/src/composables/__tests__/useFeeds.spec.js index bf6e9c3..1a7f285 100644 --- a/vue/src/composables/__tests__/useFeeds.spec.js +++ b/vue/src/composables/__tests__/useFeeds.spec.js @@ -13,7 +13,7 @@ class FakeIntersectionObserver { vi.stubGlobal('IntersectionObserver', FakeIntersectionObserver) describe('useFeeds', () => { - const { feeds, showMessage, message, showModal, fetchData, sync, getReadable, setInitialLoad, handleIntersection, consumeScrollCorrection } = useFeeds() + const { feeds, showMessage, message, showModal, fetchData, sync, getReadable, setInitialLoad, handleIntersection } = useFeeds() beforeEach(() => { localStorage.setItem('user-token', 'test-token') @@ -117,36 +117,6 @@ describe('useFeeds', () => { setInitialLoad(false) }) - it('flags a scroll correction when marking an article read repositions the list', async () => { - // Regression coverage for the header-compacting glitch: AppNav's scroll - // handler needs a way to tell "the page moved because content was - // removed" apart from a real user scroll. jsdom's getBoundingClientRect() - // defaults every element's top to 0, so any topbarHeight > 0 exercises - // the same "first article now sits above the header" branch that fires - // in a real browser. - feeds.value = [{ id: 101, title: 'First' }, { id: 102, title: 'Second' }] - setInitialLoad(true) - axios.put.mockResolvedValue({ status: 200 }) - - const observeDiv = document.createElement('div') - observeDiv.className = 'observe' - document.body.appendChild(observeDiv) - - expect(consumeScrollCorrection()).toBe(false) - - await handleIntersection([ - { isIntersecting: false, boundingClientRect: { y: -10 }, target: { id: '0' } }, - ], 60) - await flushPromises() - - expect(consumeScrollCorrection()).toBe(true) - // Consume-once: asking again without another correction reports false. - expect(consumeScrollCorrection()).toBe(false) - - document.body.removeChild(observeDiv) - setInitialLoad(false) - }) - it('strips leftover embedded-video placeholder headings', async () => { feeds.value = [{ id: 1, diff --git a/vue/src/composables/useFeeds.js b/vue/src/composables/useFeeds.js index 9ec5f98..5ecf050 100644 --- a/vue/src/composables/useFeeds.js +++ b/vue/src/composables/useFeeds.js @@ -14,13 +14,6 @@ const layout = ref(localStorage.getItem('layout') || 'list') // 'list' | 'cards' let observer; // Declare observer outside the setup function let initialLoad = false -// Set right before the scroll-position correction below, so AppNav's -// scroll-driven header compacting can tell "the page just moved because -// content was removed" apart from a real user scroll — otherwise every -// article marked read (which resets scrollY toward the top of the -// now-shorter list) would look identical to the user manually scrolling -// back to the top. -let scrollCorrectionPending = false export function authHeaders() { return { @@ -272,7 +265,6 @@ function handleIntersection(entries, topbarHeight = 0) { if (first) { const top = first.getBoundingClientRect().top if (top < topbarHeight) { - scrollCorrectionPending = true window.scrollBy(0, top - topbarHeight) } } @@ -280,17 +272,6 @@ function handleIntersection(entries, topbarHeight = 0) { }) } -// Consume-once: returns whether a scroll-position correction is pending -// (and clears it), so a caller can tell this scroll event apart from a -// real user scroll without the flag lingering into later, unrelated ones. -function consumeScrollCorrection() { - if (scrollCorrectionPending) { - scrollCorrectionPending = false - return true - } - return false -} - function disconnectObserver() { if (observer) { observer.disconnect() @@ -405,7 +386,6 @@ export function useFeeds() { markAllRead, showMessageForXSeconds, setupIntersectionObserver, - consumeScrollCorrection, disconnectObserver, setInitialLoad, handleIntersection,