-
Notifications
You must be signed in to change notification settings - Fork 639
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.
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
Dapp browser: disable tab closing for empty state #5573
Conversation
const opacity = tabViewVisible?.value || !isActiveTab ? interpolatedOpacity : withTiming(0, TIMING_CONFIGS.fastFadeConfig); | ||
|
||
const opacity = | ||
(multipleTabsOpen || !isOnHomepage) && (tabViewVisible?.value || !isActiveTab) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might need diff animation config here once we add animation for going from multiple tabs -> single tab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, in the swipeToCloseTabGestureHandler
though I'd remove all of the early returns you added and instead modify onEnd
like this, so that it rebounds instead of blocking the gesture:
onEnd: (e, ctx: { startX?: number }) => {
if (!tabViewVisible?.value) return;
const xDelta = e.absoluteX - (ctx.startX || 0);
setNativeProps(scrollViewRef, { scrollEnabled: !!tabViewVisible?.value });
const isBeyondDismissThreshold = (xDelta < -(TAB_VIEW_COLUMN_WIDTH / 2 + 20) && e.velocityX <= 0);
const isFastLeftwardSwipe = e.velocityX < -500;
if ((isBeyondDismissThreshold || isFastLeftwardSwipe) && !isEmptyState) {
const xDestination = -Math.min(Math.max(deviceWidth * 1.25, Math.abs(e.velocityX * 0.3)), 1000);
gestureX.value = withTiming(xDestination, TIMING_CONFIGS.tabPressConfig, () => {
runOnJS(closeTab)(tabId);
gestureScale.value = 0;
gestureX.value = 0;
gestureY.value = 0;
ctx.startX = undefined;
});
} else {
gestureScale.value = withTiming(1, TIMING_CONFIGS.tabPressConfig);
gestureX.value = withTiming(0, TIMING_CONFIGS.tabPressConfig);
gestureY.value = withTiming(0, TIMING_CONFIGS.tabPressConfig);
ctx.startX = undefined;
}
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good aside from my previous comment
@@ -616,13 +617,13 @@ export const BrowserTab = React.memo(function BrowserTab({ tabId, tabIndex, inje | |||
|
|||
const swipeToCloseTabGestureHandler = useAnimatedGestureHandler<PanGestureHandlerGestureEvent>({ | |||
onStart: (_, ctx: { startX?: number }) => { | |||
if (!tabViewVisible?.value) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these early returns should be removed once we fix the disabling of the gesture handlers outside of the tab view
probably currently allows the tab to get stuck if while dragging a tab you exit the tab view
we could even remove the early return in onEnd
now and add a !!tabViewVisible?.value
condition where I added !isEmptyTab
:
onEnd: (e, ctx: { startX?: number }) => {
const xDelta = e.absoluteX - (ctx.startX || 0);
setNativeProps(scrollViewRef, { scrollEnabled: !!tabViewVisible?.value });
const isBeyondDismissThreshold = (xDelta < -(TAB_VIEW_COLUMN_WIDTH / 2 + 20) && e.velocityX <= 0);
const isFastLeftwardSwipe = e.velocityX < -500;
const shouldDismiss = !!tabViewVisible?.value && !isEmptyState && (isBeyondDismissThreshold || isFastLeftwardSwipe);
if (shouldDismiss) {
// rest the same
✅✅ |
* 'develop' of github.com:rainbow-me/rainbow: Only hold the active tab ref in BrowserContext (#5579) Dapp browser: disable tab closing for empty state (#5573) Browser: fix ref assignment, back/forward navigation (#5578) Browser: fully eliminate reloading issues (#5576) browser: static trending dapps (#5561) bump swaps sdk (#5574) fix gitignore (#5571) bump (#5570) Fix browser context menu not updating (#5569) ⚡️ Fast browser (#5566) [APP-1049]: (feat): Backups V2 (#5310) fix: search by contract address (#5563)
…e-changes * 'develop' of github.com:rainbow-me/rainbow: (44 commits) allow open in new tab (#5610) added warning for unknown price impact (#5597) fix cloudflare protection (#5609) improve type checking on web preferences (#5607) fix scrolltoindex firing on last card dismissal (#5606) make account network switcher work (#5604) Dapp browser fixes (#5596) Fix close tab btn (#5598) browser: add account context menu (#5603) Fix instant screenshot setting (#5602) swaps: bump sdk (#5583) Browser: tab transitions, state update queue (#5582) audit: undici (#5594) [SWAPS V2]: Add token search logic and ability to select assets (#5547) swaps v2 gas (#5526) tx sim: other natives (#5585) Brody/bump 1.9.21 3 (#5588) fix sheet bg (#5590) Only hold the active tab ref in BrowserContext (#5579) Dapp browser: disable tab closing for empty state (#5573) ...
Fixes APP-####
What changed (plus any additional context for devs)
Screen recordings / screenshots
What to test