diff --git a/vue/src/components/AppNav.vue b/vue/src/components/AppNav.vue index 8f80a75..4b73ea8 100644 --- a/vue/src/components/AppNav.vue +++ b/vue/src/components/AppNav.vue @@ -6,7 +6,7 @@ import Modal from './modal/AddUrl.vue' const router = useRouter() const route = useRoute() -const { sync, showModal, viewMode, toggleViewMode, layout, toggleLayout, markAllRead, feedFilter, feedTitles, feedUnreadCounts, setFeedFilter, unreadCount, lastProgrammaticScroll } = useFeeds() +const { sync, showModal, viewMode, toggleViewMode, layout, toggleLayout, markAllRead, feedFilter, feedTitles, setFeedFilter, unreadCount, displayedFeedUnreadCounts, refreshUnreadDisplay, lastProgrammaticScroll } = useFeeds() const headerRef = ref(null) @@ -27,15 +27,25 @@ const REVEAL_THRESHOLD = 12 // px of accumulated travel before toggling const PROGRAMMATIC_SUPPRESS_MS = 300 let lastY = 0 let accumulated = 0 +// Cached in onMounted() below instead of re-read via headerRef.offsetHeight +// on every scroll event. The header is a fixed size (see onMounted), so +// re-measuring it here was pure waste — worse, a DevTools performance trace +// showed that offsetHeight read landing right after an unrelated DOM write +// elsewhere on the page (e.g. the article view swapping in a new article) +// forces the browser into a synchronous, forced-reflow layout pass right +// there mid-script, instead of its normal scheduled one. Stays 0 until +// onMounted() below sets it (before the scroll listener is attached, so +// onScroll() never runs against the unset value) — keep that ordering if +// this ever changes. +let headerHeight = 0 function onScroll() { const y = Math.max(0, window.scrollY) - const headerH = headerRef.value?.offsetHeight ?? 0 // Always reveal near the very top, and keep it visible while the menu is // open (the dropdown is anchored to the header, so hiding it would slide the // open menu off-screen). - if (y <= headerH || menuOpen.value) { + if (y <= headerHeight || menuOpen.value) { hidden.value = false accumulated = 0 lastY = y @@ -71,13 +81,16 @@ function onScrollRaf() { onMounted(() => { // Drives #app's padding-top / RssFeeds' scroll-margin-top so content below // the fixed header isn't hidden behind it at scroll position 0. The header is - // a fixed size, so this is measured once on mount and never changes. + // a fixed size, so this is measured once on mount and never changes — and + // onScroll() reuses this same measurement instead of re-reading it. const h = headerRef.value?.getBoundingClientRect().height ?? 0 document.documentElement.style.setProperty('--app-nav-height', `${h}px`) + headerHeight = h lastY = Math.max(0, window.scrollY) window.addEventListener('scroll', onScrollRaf, { passive: true }) document.addEventListener('click', onDocumentClick) + refreshUnreadDisplay() }) onUnmounted(() => { @@ -152,10 +165,11 @@ function handleToggleLayout() { class="app-nav__filter" :value="feedFilter ?? ''" aria-label="Filter by feed" + @focus="refreshUnreadDisplay" @change="setFeedFilter($event.target.value || null)" > - + +

+

+
@@ -381,46 +367,6 @@ onMounted(async () => { max-width: 720px; } -/* Slide-in/out when paging between articles (swipe or nav buttons). The - outgoing article is pulled out of flex flow during its leave transition so - the incoming one can take its place immediately, instead of the two - stacking and doubling the page height for the transition's duration. */ -.slide-next-enter-active, -.slide-next-leave-active, -.slide-prev-enter-active, -.slide-prev-leave-active { - transition: transform 0.28s ease, opacity 0.28s ease; -} - -.slide-next-leave-active, -.slide-prev-leave-active { - position: absolute; - left: 0; - right: 0; - margin-left: auto; - margin-right: auto; -} - -.slide-next-enter-from { - transform: translateX(100%); - opacity: 0; -} - -.slide-next-leave-to { - transform: translateX(-100%); - opacity: 0; -} - -.slide-prev-enter-from { - transform: translateX(-100%); - opacity: 0; -} - -.slide-prev-leave-to { - transform: translateX(100%); - opacity: 0; -} - .article-feature__source { margin: 0 0 0.5em; padding: 0 1rem; diff --git a/vue/src/components/__tests__/AppNav.spec.js b/vue/src/components/__tests__/AppNav.spec.js index a88b7c6..5217029 100644 --- a/vue/src/components/__tests__/AppNav.spec.js +++ b/vue/src/components/__tests__/AppNav.spec.js @@ -306,9 +306,18 @@ describe('AppNav', () => { // Reset scroll position so each mount's lastY baseline starts at 0. Object.defineProperty(window, 'scrollY', { value: 0, configurable: true, writable: true }) vi.stubGlobal('requestAnimationFrame', (cb) => { cb(); return 0 }) - // offsetHeight is 0 in jsdom; give the header a real height so the - // "near the top" guard (scrollY <= headerH) has something to compare to. - vi.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockReturnValue(50) + // getBoundingClientRect() is all-zero in jsdom; give the header a real + // height so onMounted()'s one-time measurement (which onScroll() then + // reuses instead of re-reading offsetHeight — see AppNav.vue) has + // something to compare against for the "near the top" guard. This is a + // prototype-wide spy (scoped to this describe block only, restored in + // afterEach), so it returns a full DOMRect-shaped object rather than a + // partial one — no test here happens to call getBoundingClientRect() on + // anything but the header, but a bare `{ height: 50 }` would silently + // hand back `undefined` for top/left/width/etc. to any code that did. + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({ + height: 50, width: 0, top: 0, left: 0, right: 0, bottom: 0, x: 0, y: 0, toJSON() {}, + }) }) afterEach(() => { diff --git a/vue/src/composables/__tests__/useFeeds.spec.js b/vue/src/composables/__tests__/useFeeds.spec.js index 2211761..61b9b0e 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, allItems, feedFilter, feedTitles, feedUnreadCounts, unreadCount, setFeedFilter, showMessage, message, showModal, fetchData, sync, getReadable, setInitialLoad, handleIntersection, markAllRead, setupIntersectionObserver } = useFeeds() + const { feeds, allItems, feedFilter, feedTitles, feedUnreadCounts, unreadCount, displayedFeedUnreadCounts, refreshUnreadDisplay, setFeedFilter, showMessage, message, showModal, fetchData, sync, getReadable, setInitialLoad, handleIntersection, markAllRead, setupIntersectionObserver } = useFeeds() beforeEach(() => { localStorage.setItem('user-token', 'test-token') @@ -119,6 +119,32 @@ describe('useFeeds', () => { setInitialLoad(false) }) + it('refreshes the filter dropdown\'s displayed per-feed counts when list-view scrolling marks an article read', async () => { + // displayedFeedUnreadCounts is a frozen snapshot of feedUnreadCounts (see + // useFeeds.js) — AppNav's filter 's per-feed option counts, instead of binding +// feedUnreadCounts directly — by design, this stays frozen while paging +// through article view (markCurrentArticleRead marks one article read per +// page turn, which would otherwise recompute it live on every single swipe). +// The header's own unread badge stays bound to the live unreadCount (it +// should update in real time regardless of view). Everywhere else that marks +// articles read — list-view scrolling (handleIntersection), opening/changing +// the filter, syncing, mark-all-read, leaving article view — refreshes this +// snapshot immediately, so it only ever looks stale during article-view +// paging itself. +const displayedFeedUnreadCounts = ref({}) +function refreshUnreadDisplay() { + displayedFeedUnreadCounts.value = feedUnreadCounts.value +} const message = ref('') const showModal = ref(false) const viewMode = ref(localStorage.getItem('viewMode') || 'list') // 'list' | 'article' — toggled from the hamburger menu, persisted per device @@ -242,6 +257,7 @@ async function setFeedFilter(title) { feedFilter.value = title // null for "All feeds" currentIndex.value = 0 applyFilter() + refreshUnreadDisplay() await nextTick() setupIntersectionObserver() } @@ -259,6 +275,7 @@ const fetchData = async () => { items.sort((a, b) => b.timestamp.localeCompare(a.timestamp)); allItems.value = items; applyFilter(); + refreshUnreadDisplay(); await nextTick(); setupIntersectionObserver(); } catch (error) { @@ -346,6 +363,7 @@ function handleIntersection(entries, topbarHeight = 0) { feeds.value = feeds.value.filter(feed => !readIds.has(feed.id)) // Mirror into the master so cleared filters don't resurrect read articles. allItems.value = allItems.value.filter(feed => !readIds.has(feed.id)) + refreshUnreadDisplay() for (const feed of readFeeds) { markRead(feed.id) @@ -392,6 +410,7 @@ async function markAllRead() { // is active) — drop exactly those from the master too. allItems.value = allItems.value.filter(feed => !readIds.has(feed.id)) currentIndex.value = 0 + refreshUnreadDisplay() // markRead swallows its own errors, so Promise.all can't reject here. await Promise.all(ids.map(id => markRead(id))) showMessageForXSeconds('All articles marked as read.', 5) @@ -418,6 +437,7 @@ async function leaveArticleView() { currentIndex.value = 0 viewMode.value = 'list' localStorage.setItem('viewMode', viewMode.value) + refreshUnreadDisplay() // The v-if on the list container tears down and recreates all .observe DOM // nodes when switching views, so the intersection observer must be // re-pointed at the new elements after Vue has finished rendering. @@ -479,6 +499,8 @@ export function useFeeds() { feedTitles, feedUnreadCounts, unreadCount, + displayedFeedUnreadCounts, + refreshUnreadDisplay, setFeedFilter, showMessage, message,