Skip to content
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

Hide/show header and footer without re-renders, take two #1849

Merged
merged 4 commits into from
Nov 9, 2023

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 9, 2023

Re-doing the fix from #1691 which regressed in #1676.

This time it's easier to fix at the source. Now that we've moved the shell state into React, I've changed the source of truth to be an Animated value. We can change it directly and make all animations derived from it.

In the future, we should also change this to update with scroll but for now I left it discrete.

Verification

// ReactNativeRenderer-dev.js
+let RENDERS = []
    
+setInterval(() => {
+  console.log('new renders:', RENDERS.join(', '))
+  RENDERS = []
+}, 1000)
    
function renderWithHooks(
  current,
  workInProgress,
  Component,
  props,
  secondArg,
  nextRenderLanes
) {
+  RENDERS.push(Component.name)

Before

before.mov

After

after.mov

Impact

Low/mid-range Android.

Before

IMG_0816.mov

After

IMG_0819.mov

Test Plan

Verified these elements hide/show as expected:

  • Top bar
  • Bottom bar
  • FAB
  • Load latest
  • Sticky "reply to thread"

@gaearon gaearon requested a review from pfrazee November 9, 2023 00:21
const setContext = React.createContext<SetContext>((_: boolean) => {})

export function Provider({children}: React.PropsWithChildren<{}>) {
const [state, setState] = React.useState(false)
const mode = useSharedValue(false)
const setMode = React.useCallback(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the setter so that writes are centralized and easy to trace. Technically code can screw around with .value but we can catch that in review.

style={[
styles.prompt,
fabMinimalShellTransform,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could rename this since this isn't technically FAB, I think it's fine though.

Copy link
Collaborator

@pfrazee pfrazee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@@ -38,7 +38,6 @@ export const FeedsTabBar = observer(function FeedsTabBarImpl(
pal.border,
styles.tabBar,
headerMinimalShellTransform,
minimalShellMode && styles.disabled,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved these into the animated styles so that we don't expose the raw value.

@gaearon gaearon merged commit 82059b7 into main Nov 9, 2023
4 checks passed
@gaearon gaearon deleted the min-shell-reanimated branch November 9, 2023 00:25
estrattonbailey added a commit that referenced this pull request Nov 9, 2023
* origin:
  Fix tab alignment on the web (#1857)
  Show tabs when swiping feeds (#1856)
  Sync top/bottom bar disappearance to the scroll (#1855)
  Hotfix internationalization on mobile (#1854)
  Internationalization & localization (#1822)
  Hide/show header and footer without re-renders, take two (#1849)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants