Remove onPageSelecting event

It's difficult to tell what exactly it's supposed to represent, and in practice it's not really used aside from logging. Let's rip it out for now to keep other changes simpler.
This commit is contained in:
Dan Abramov
2024-11-26 20:01:53 +00:00
parent 972fb887ec
commit 06d03fff94
6 changed files with 12 additions and 63 deletions
-6
View File
@@ -80,12 +80,6 @@ export type LogEvents = {
feedUrl: string feedUrl: string
feedType: string feedType: string
index: number index: number
reason:
| 'focus'
| 'tabbar-click'
| 'pager-swipe'
| 'desktop-sidebar-click'
| 'starter-pack-initial-feed'
} }
'feed:endReached': { 'feed:endReached': {
feedUrl: string feedUrl: string
+4 -20
View File
@@ -6,16 +6,12 @@ import PagerView, {
PageScrollStateChangedNativeEvent, PageScrollStateChangedNativeEvent,
} from 'react-native-pager-view' } from 'react-native-pager-view'
import {LogEvents} from '#/lib/statsig/events'
import {atoms as a, native} from '#/alf' import {atoms as a, native} from '#/alf'
export type PageSelectedEvent = PagerViewOnPageSelectedEvent export type PageSelectedEvent = PagerViewOnPageSelectedEvent
export interface PagerRef { export interface PagerRef {
setPage: ( setPage: (index: number) => void
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => void
} }
export interface RenderTabBarFnProps { export interface RenderTabBarFnProps {
@@ -29,10 +25,6 @@ interface Props {
initialPage?: number initialPage?: number
renderTabBar: RenderTabBarFn renderTabBar: RenderTabBarFn
onPageSelected?: (index: number) => void onPageSelected?: (index: number) => void
onPageSelecting?: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => void
onPageScrollStateChanged?: ( onPageScrollStateChanged?: (
scrollState: 'idle' | 'dragging' | 'settling', scrollState: 'idle' | 'dragging' | 'settling',
) => void ) => void
@@ -46,7 +38,6 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
renderTabBar, renderTabBar,
onPageScrollStateChanged, onPageScrollStateChanged,
onPageSelected, onPageSelected,
onPageSelecting,
testID, testID,
}: React.PropsWithChildren<Props>, }: React.PropsWithChildren<Props>,
ref, ref,
@@ -58,12 +49,8 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
const pagerView = React.useRef<PagerView>(null) const pagerView = React.useRef<PagerView>(null)
React.useImperativeHandle(ref, () => ({ React.useImperativeHandle(ref, () => ({
setPage: ( setPage: (index: number) => {
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => {
pagerView.current?.setPage(index) pagerView.current?.setPage(index)
onPageSelecting?.(index, reason)
}, },
})) }))
@@ -92,14 +79,12 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
// -prf // -prf
if (scrollState.current === 'settling') { if (scrollState.current === 'settling') {
if (lastDirection.current === -1 && offset < lastOffset.current) { if (lastDirection.current === -1 && offset < lastOffset.current) {
onPageSelecting?.(position, 'pager-swipe')
setSelectedPage(position) setSelectedPage(position)
lastDirection.current = 0 lastDirection.current = 0
} else if ( } else if (
lastDirection.current === 1 && lastDirection.current === 1 &&
offset > lastOffset.current offset > lastOffset.current
) { ) {
onPageSelecting?.(position + 1, 'pager-swipe')
setSelectedPage(position + 1) setSelectedPage(position + 1)
lastDirection.current = 0 lastDirection.current = 0
} }
@@ -112,7 +97,7 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
} }
lastOffset.current = offset lastOffset.current = offset
}, },
[lastOffset, lastDirection, onPageSelecting], [lastOffset, lastDirection],
) )
const handlePageScrollStateChanged = React.useCallback( const handlePageScrollStateChanged = React.useCallback(
@@ -126,9 +111,8 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
const onTabBarSelect = React.useCallback( const onTabBarSelect = React.useCallback(
(index: number) => { (index: number) => {
pagerView.current?.setPage(index) pagerView.current?.setPage(index)
onPageSelecting?.(index, 'tabbar-click')
}, },
[pagerView, onPageSelecting], [pagerView],
) )
return ( return (
+5 -15
View File
@@ -2,7 +2,6 @@ import React from 'react'
import {View} from 'react-native' import {View} from 'react-native'
import {flushSync} from 'react-dom' import {flushSync} from 'react-dom'
import {LogEvents} from '#/lib/statsig/events'
import {s} from '#/lib/styles' import {s} from '#/lib/styles'
export interface RenderTabBarFnProps { export interface RenderTabBarFnProps {
@@ -16,10 +15,6 @@ interface Props {
initialPage?: number initialPage?: number
renderTabBar: RenderTabBarFn renderTabBar: RenderTabBarFn
onPageSelected?: (index: number) => void onPageSelected?: (index: number) => void
onPageSelecting?: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => void
} }
export const Pager = React.forwardRef(function PagerImpl( export const Pager = React.forwardRef(function PagerImpl(
{ {
@@ -27,7 +22,6 @@ export const Pager = React.forwardRef(function PagerImpl(
initialPage = 0, initialPage = 0,
renderTabBar, renderTabBar,
onPageSelected, onPageSelected,
onPageSelecting,
}: React.PropsWithChildren<Props>, }: React.PropsWithChildren<Props>,
ref, ref,
) { ) {
@@ -36,16 +30,13 @@ export const Pager = React.forwardRef(function PagerImpl(
const anchorRef = React.useRef(null) const anchorRef = React.useRef(null)
React.useImperativeHandle(ref, () => ({ React.useImperativeHandle(ref, () => ({
setPage: ( setPage: (index: number) => {
index: number, onTabBarSelect(index)
reason: LogEvents['home:feedDisplayed']['reason'],
) => {
onTabBarSelect(index, reason)
}, },
})) }))
const onTabBarSelect = React.useCallback( const onTabBarSelect = React.useCallback(
(index: number, reason: LogEvents['home:feedDisplayed']['reason']) => { (index: number) => {
const scrollY = window.scrollY const scrollY = window.scrollY
// We want to determine if the tabbar is already "sticking" at the top (in which // We want to determine if the tabbar is already "sticking" at the top (in which
// case we should preserve and restore scroll), or if it is somewhere below in the // case we should preserve and restore scroll), or if it is somewhere below in the
@@ -64,7 +55,6 @@ export const Pager = React.forwardRef(function PagerImpl(
flushSync(() => { flushSync(() => {
setSelectedPage(index) setSelectedPage(index)
onPageSelected?.(index) onPageSelected?.(index)
onPageSelecting?.(index, reason)
}) })
if (isSticking) { if (isSticking) {
const restoredScrollY = scrollYs.current[index] const restoredScrollY = scrollYs.current[index]
@@ -75,7 +65,7 @@ export const Pager = React.forwardRef(function PagerImpl(
} }
} }
}, },
[selectedPage, setSelectedPage, onPageSelected, onPageSelecting], [selectedPage, setSelectedPage, onPageSelected],
) )
return ( return (
@@ -83,7 +73,7 @@ export const Pager = React.forwardRef(function PagerImpl(
{renderTabBar({ {renderTabBar({
selectedPage, selectedPage,
tabBarAnchor: <View ref={anchorRef} />, tabBarAnchor: <View ref={anchorRef} />,
onSelect: e => onTabBarSelect(e, 'tabbar-click'), onSelect: e => onTabBarSelect(e),
})} })}
{React.Children.map(children, (child, i) => ( {React.Children.map(children, (child, i) => (
<View style={selectedPage === i ? s.flex1 : s.hidden} key={`page-${i}`}> <View style={selectedPage === i ? s.flex1 : s.hidden} key={`page-${i}`}>
-5
View File
@@ -182,17 +182,12 @@ export const PagerWithHeader = React.forwardRef<PagerRef, PagerWithHeaderProps>(
[onPageSelected, setCurrentPage], [onPageSelected, setCurrentPage],
) )
const onPageSelecting = React.useCallback((index: number) => {
setCurrentPage(index)
}, [])
return ( return (
<Pager <Pager
ref={ref} ref={ref}
testID={testID} testID={testID}
initialPage={initialPage} initialPage={initialPage}
onPageSelected={onPageSelectedInner} onPageSelected={onPageSelectedInner}
onPageSelecting={onPageSelecting}
renderTabBar={renderTabBar}> renderTabBar={renderTabBar}>
{toArray(children) {toArray(children)
.filter(Boolean) .filter(Boolean)
@@ -75,17 +75,12 @@ export const PagerWithHeader = React.forwardRef<PagerRef, PagerWithHeaderProps>(
[onPageSelected, setCurrentPage], [onPageSelected, setCurrentPage],
) )
const onPageSelecting = React.useCallback((index: number) => {
setCurrentPage(index)
}, [])
return ( return (
<Pager <Pager
ref={ref} ref={ref}
testID={testID} testID={testID}
initialPage={initialPage} initialPage={initialPage}
onPageSelected={onPageSelectedInner} onPageSelected={onPageSelectedInner}
onPageSelecting={onPageSelecting}
renderTabBar={renderTabBar}> renderTabBar={renderTabBar}>
{toArray(children) {toArray(children)
.filter(Boolean) .filter(Boolean)
+3 -12
View File
@@ -11,7 +11,7 @@ import {
HomeTabNavigatorParams, HomeTabNavigatorParams,
NativeStackScreenProps, NativeStackScreenProps,
} from '#/lib/routes/types' } from '#/lib/routes/types'
import {logEvent, LogEvents} from '#/lib/statsig/statsig' import {logEvent} from '#/lib/statsig/statsig'
import {isWeb} from '#/platform/detection' import {isWeb} from '#/platform/detection'
import {emitSoftReset} from '#/state/events' import {emitSoftReset} from '#/state/events'
import {SavedFeedSourceInfo, usePinnedFeedsInfos} from '#/state/queries/feed' import {SavedFeedSourceInfo, usePinnedFeedsInfos} from '#/state/queries/feed'
@@ -121,7 +121,7 @@ function HomeScreenReady({
// This is supposed to only happen on the web when you use the right nav. // This is supposed to only happen on the web when you use the right nav.
if (selectedIndex !== lastPagerReportedIndexRef.current) { if (selectedIndex !== lastPagerReportedIndexRef.current) {
lastPagerReportedIndexRef.current = selectedIndex lastPagerReportedIndexRef.current = selectedIndex
pagerRef.current?.setPage(selectedIndex, 'desktop-sidebar-click') pagerRef.current?.setPage(selectedIndex)
} }
}, [selectedIndex]) }, [selectedIndex])
@@ -158,21 +158,13 @@ function HomeScreenReady({
const feed = allFeeds[index] const feed = allFeeds[index]
setSelectedFeed(feed) setSelectedFeed(feed)
lastPagerReportedIndexRef.current = index lastPagerReportedIndexRef.current = index
},
[setDrawerSwipeDisabled, setSelectedFeed, setMinimalShellMode, allFeeds],
)
const onPageSelecting = React.useCallback(
(index: number, reason: LogEvents['home:feedDisplayed']['reason']) => {
const feed = allFeeds[index]
logEvent('home:feedDisplayed', { logEvent('home:feedDisplayed', {
index, index,
feedType: feed.split('|')[0], feedType: feed.split('|')[0],
feedUrl: feed, feedUrl: feed,
reason,
}) })
}, },
[allFeeds], [setDrawerSwipeDisabled, setSelectedFeed, setMinimalShellMode, allFeeds],
) )
const onPressSelected = React.useCallback(() => { const onPressSelected = React.useCallback(() => {
@@ -228,7 +220,6 @@ function HomeScreenReady({
ref={pagerRef} ref={pagerRef}
testID="homeScreen" testID="homeScreen"
initialPage={selectedIndex} initialPage={selectedIndex}
onPageSelecting={onPageSelecting}
onPageSelected={onPageSelected} onPageSelected={onPageSelected}
onPageScrollStateChanged={onPageScrollStateChanged} onPageScrollStateChanged={onPageScrollStateChanged}
renderTabBar={renderTabBar}> renderTabBar={renderTabBar}>