Skip to content

Commit

Permalink
[Nicer Tabs] Fork TabBar, simplify Pager (#6762)
Browse files Browse the repository at this point in the history
* Fork TabBar.web.tsx

* Trim dead code from both forks

* 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.

* Remove early onPageSelected call

It was added to try to do some work eagerly when we're sure which way the scroll is snapping. This is not necessarily a good idea though. It schedules a potentially expensive re-render right during the deceleration animation, which is not great. Whatever we're optimizing there, we should optimize smarter (e.g. prewarm just the network call). The other thing it used to help with is triggering the pager header autoscroll earlier. But we're going to rewrite that part differently anyway so that's not relevant either.

* Prune more dead code from the native version

We'll have to revisit this when adding tablet support but for now I'd prefer to remove a codepath that is not being tested or ever run.

* Use regular ScrollView on native

The Draggable thing was needed for web-only behavior so we can drop it in the native fork.
  • Loading branch information
gaearon authored Dec 3, 2024
1 parent 996871d commit 5a313c2
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 199 deletions.
6 changes: 0 additions & 6 deletions src/lib/statsig/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ export type LogEvents = {
feedUrl: string
feedType: string
index: number
reason:
| 'focus'
| 'tabbar-click'
| 'pager-swipe'
| 'desktop-sidebar-click'
| 'starter-pack-initial-feed'
}
'feed:endReached': {
feedUrl: string
Expand Down
70 changes: 5 additions & 65 deletions src/view/com/pager/Pager.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
import React, {forwardRef} from 'react'
import {View} from 'react-native'
import PagerView, {
PagerViewOnPageScrollEvent,
PagerViewOnPageSelectedEvent,
PageScrollStateChangedNativeEvent,
} from 'react-native-pager-view'

import {LogEvents} from '#/lib/statsig/events'
import {atoms as a, native} from '#/alf'

export type PageSelectedEvent = PagerViewOnPageSelectedEvent

export interface PagerRef {
setPage: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => void
setPage: (index: number) => void
}

export interface RenderTabBarFnProps {
Expand All @@ -29,10 +24,6 @@ interface Props {
initialPage?: number
renderTabBar: RenderTabBarFn
onPageSelected?: (index: number) => void
onPageSelecting?: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => void
onPageScrollStateChanged?: (
scrollState: 'idle' | 'dragging' | 'settling',
) => void
Expand All @@ -46,24 +37,16 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
renderTabBar,
onPageScrollStateChanged,
onPageSelected,
onPageSelecting,
testID,
}: React.PropsWithChildren<Props>,
ref,
) {
const [selectedPage, setSelectedPage] = React.useState(0)
const lastOffset = React.useRef(0)
const lastDirection = React.useRef(0)
const scrollState = React.useRef('')
const pagerView = React.useRef<PagerView>(null)

React.useImperativeHandle(ref, () => ({
setPage: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => {
setPage: (index: number) => {
pagerView.current?.setPage(index)
onPageSelecting?.(index, reason)
},
}))

Expand All @@ -75,60 +58,18 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
[setSelectedPage, onPageSelected],
)

const onPageScroll = React.useCallback(
(e: PagerViewOnPageScrollEvent) => {
const {position, offset} = e.nativeEvent
if (offset === 0) {
// offset hits 0 in some awkward spots so we ignore it
return
}
// NOTE
// we want to call `onPageSelecting` as soon as the scroll-gesture
// enters the "settling" phase, which means the user has released it
// we can't infer directionality from the scroll information, so we
// track the offset changes. if the offset delta is consistent with
// the existing direction during the settling phase, we can say for
// certain where it's going and can fire
// -prf
if (scrollState.current === 'settling') {
if (lastDirection.current === -1 && offset < lastOffset.current) {
onPageSelecting?.(position, 'pager-swipe')
setSelectedPage(position)
lastDirection.current = 0
} else if (
lastDirection.current === 1 &&
offset > lastOffset.current
) {
onPageSelecting?.(position + 1, 'pager-swipe')
setSelectedPage(position + 1)
lastDirection.current = 0
}
} else {
if (offset < lastOffset.current) {
lastDirection.current = -1
} else if (offset > lastOffset.current) {
lastDirection.current = 1
}
}
lastOffset.current = offset
},
[lastOffset, lastDirection, onPageSelecting],
)

const handlePageScrollStateChanged = React.useCallback(
(e: PageScrollStateChangedNativeEvent) => {
scrollState.current = e.nativeEvent.pageScrollState
onPageScrollStateChanged?.(e.nativeEvent.pageScrollState)
},
[scrollState, onPageScrollStateChanged],
[onPageScrollStateChanged],
)

const onTabBarSelect = React.useCallback(
(index: number) => {
pagerView.current?.setPage(index)
onPageSelecting?.(index, 'tabbar-click')
},
[pagerView, onPageSelecting],
[pagerView],
)

return (
Expand All @@ -142,8 +83,7 @@ export const Pager = forwardRef<PagerRef, React.PropsWithChildren<Props>>(
style={[a.flex_1]}
initialPage={initialPage}
onPageScrollStateChanged={handlePageScrollStateChanged}
onPageSelected={onPageSelectedInner}
onPageScroll={onPageScroll}>
onPageSelected={onPageSelectedInner}>
{children}
</PagerView>
</View>
Expand Down
20 changes: 5 additions & 15 deletions src/view/com/pager/Pager.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react'
import {View} from 'react-native'
import {flushSync} from 'react-dom'

import {LogEvents} from '#/lib/statsig/events'
import {s} from '#/lib/styles'

export interface RenderTabBarFnProps {
Expand All @@ -16,18 +15,13 @@ interface Props {
initialPage?: number
renderTabBar: RenderTabBarFn
onPageSelected?: (index: number) => void
onPageSelecting?: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => void
}
export const Pager = React.forwardRef(function PagerImpl(
{
children,
initialPage = 0,
renderTabBar,
onPageSelected,
onPageSelecting,
}: React.PropsWithChildren<Props>,
ref,
) {
Expand All @@ -36,16 +30,13 @@ export const Pager = React.forwardRef(function PagerImpl(
const anchorRef = React.useRef(null)

React.useImperativeHandle(ref, () => ({
setPage: (
index: number,
reason: LogEvents['home:feedDisplayed']['reason'],
) => {
onTabBarSelect(index, reason)
setPage: (index: number) => {
onTabBarSelect(index)
},
}))

const onTabBarSelect = React.useCallback(
(index: number, reason: LogEvents['home:feedDisplayed']['reason']) => {
(index: number) => {
const scrollY = window.scrollY
// 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
Expand All @@ -64,7 +55,6 @@ export const Pager = React.forwardRef(function PagerImpl(
flushSync(() => {
setSelectedPage(index)
onPageSelected?.(index)
onPageSelecting?.(index, reason)
})
if (isSticking) {
const restoredScrollY = scrollYs.current[index]
Expand All @@ -75,15 +65,15 @@ export const Pager = React.forwardRef(function PagerImpl(
}
}
},
[selectedPage, setSelectedPage, onPageSelected, onPageSelecting],
[selectedPage, setSelectedPage, onPageSelected],
)

return (
<View style={s.hContentRegion}>
{renderTabBar({
selectedPage,
tabBarAnchor: <View ref={anchorRef} />,
onSelect: e => onTabBarSelect(e, 'tabbar-click'),
onSelect: e => onTabBarSelect(e),
})}
{React.Children.map(children, (child, i) => (
<View style={selectedPage === i ? s.flex1 : s.hidden} key={`page-${i}`}>
Expand Down
5 changes: 0 additions & 5 deletions src/view/com/pager/PagerWithHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,12 @@ export const PagerWithHeader = React.forwardRef<PagerRef, PagerWithHeaderProps>(
[onPageSelected, setCurrentPage],
)

const onPageSelecting = React.useCallback((index: number) => {
setCurrentPage(index)
}, [])

return (
<Pager
ref={ref}
testID={testID}
initialPage={initialPage}
onPageSelected={onPageSelectedInner}
onPageSelecting={onPageSelecting}
renderTabBar={renderTabBar}>
{toArray(children)
.filter(Boolean)
Expand Down
5 changes: 0 additions & 5 deletions src/view/com/pager/PagerWithHeader.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,12 @@ export const PagerWithHeader = React.forwardRef<PagerRef, PagerWithHeaderProps>(
[onPageSelected, setCurrentPage],
)

const onPageSelecting = React.useCallback((index: number) => {
setCurrentPage(index)
}, [])

return (
<Pager
ref={ref}
testID={testID}
initialPage={initialPage}
onPageSelected={onPageSelectedInner}
onPageSelecting={onPageSelecting}
renderTabBar={renderTabBar}>
{toArray(children)
.filter(Boolean)
Expand Down
Loading

0 comments on commit 5a313c2

Please sign in to comment.