Skip to content

fix: keep shadow nodes in sync#950

Merged
kirillzyusko merged 2 commits intomainfrom
fix/keep-shadow-node-in-sync
May 22, 2025
Merged

fix: keep shadow nodes in sync#950
kirillzyusko merged 2 commits intomainfrom
fix/keep-shadow-node-in-sync

Conversation

@kirillzyusko
Copy link
Copy Markdown
Owner

@kirillzyusko kirillzyusko commented May 21, 2025

📜 Description

Fixed a problem when Pressable doesn't trigger onPress callback if it's located in KeyboardStickyView (KeyboardToolbar).

💡 Motivation and Context

The problem stems from the fact that C++ shadow nodes do not know about changes caused by useKeyboardAnimation hook (we drive animation by native driver, so C++ thinks that view hasn't changed its position).

I found out that ScrollView had the same problem: facebook/react-native#36504 (comment)

Basically the fix is next:

  • after finishing the animation we dispatch onUserDrivenAnimationEnded;
  • in useAnimatedProps:
  NativeAnimatedHelper.nativeEventEmitter.addListener(
    'onUserDrivenAnimationEnded',
    data => {
      node.update();
    },
  );

Where:

  new AnimatedProps(
    props,
    () => onUpdateRef.current?.(),
    allowlistIfEnabled,
  ),

Which in turns calls:

  const isFabricNode = isFabricInstance(instance);
  if (node.__isNative) {
    // Check 2: this is an animation driven by native.
    // In native driven animations, this callback is only called once the animation completes.
    if (isFabricNode) {
      // Call `scheduleUpdate` to synchronise Fiber and Shadow tree.
      // Must not be called in Paper.
      scheduleUpdate();
    }
    return;
  }

And it triggers re-render and synchronizes C++ state with latest react state.

Probably not the best solution, but it fixes the problem. Also we have to gather all connected nodes that depends on animated value, but at the moment this data is unused:

  data => {
    node.update();
  },

So for now it's safe to skip connected nodes 🙃 (maybe later I'll revisit this)

Closes #947 #916 #588

📢 Changelog

Android

  • added keepShadowNodesInSync function extension for ThemedReactContext;
  • call keepShadowNodesInSync in the end of keyboard animation.

🤔 How Has This Been Tested?

Tested manually in FabricExample.

📸 Screenshots (if appropriate):

KeyboardStickyView KeyboardToolbar
telegram-cloud-document-2-5294411674747433771.mp4
telegram-cloud-document-2-5294411674747433770.mp4

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko self-assigned this May 21, 2025
@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🤖 android Android specific KeyboardStickyView 🩹 Anything related to KeyboardStickyView component KeyboardToolbar Anything related to KeyboardToolbar component labels May 21, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2025

📊 Package size report

Current size Target Size Difference
199476 bytes 199189 bytes 287 bytes 📈

High5Apps added a commit to High5Apps/organize-rn that referenced this pull request May 21, 2025
High5Apps added a commit to High5Apps/organize-rn that referenced this pull request May 21, 2025
@kirillzyusko kirillzyusko marked this pull request as ready for review May 22, 2025 10:47
@kirillzyusko kirillzyusko merged commit ba2e187 into main May 22, 2025
20 checks passed
@kirillzyusko kirillzyusko deleted the fix/keep-shadow-node-in-sync branch May 22, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 android Android specific 🐛 bug Something isn't working KeyboardStickyView 🩹 Anything related to KeyboardStickyView component KeyboardToolbar Anything related to KeyboardToolbar component

Projects

None yet

1 participant