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

Pressable onHoverIn onHoverOut is too slow #1861

Open
neo773 opened this issue Jun 24, 2023 · 8 comments
Open

Pressable onHoverIn onHoverOut is too slow #1861

neo773 opened this issue Jun 24, 2023 · 8 comments
Labels
bug Something isn't working Needs: Triage 🔍

Comments

@neo773
Copy link

neo773 commented Jun 24, 2023

Environment

react-native -v: 10.2.4
npm ls react-native-macos: 
discord_rn@0.0.1 /Users/neo/Documents/temp/discord_rn
└── react-native-macos@0.71.11
node -v: v18.12.1
npm -v: 8.19.2
yarn --version: 1.22.19
xcodebuild -version:
Xcode 14.3.1
Build version 14E300c

Steps to reproduce the bug

Pressable onHoverIn onHoverOut is too slow to be used in a list it has a huge latency

hover.mp4

My code

import {
  Text,
  FlatList,
} from 'react-native';
import React, { useState } from 'react';
import { users } from '../api/types/ready';
import type { private_channels } from '../api/types/ready';
import UserIcon from '../components/UserIcon/UserIcon';
import { store } from '../store/store';
import { Pressable, View } from 'react-native-macos';

const Friends = ({
  users,
  private_channels,
}: {
  users: users[];
  private_channels: private_channels[];
}) => {

  const [hoveredIndex, setHoveredIndex] = useState<number | null>(null);

  const renderItem = ({
    item: item,
    index: index,
  }: {
    item: private_channels;
    index: number;
  }) => {
    const user = users.find(user => user.id === item.recipient_ids[0]);
    if (user && !user.username.includes('Deleted User')) {
      return (
        <Pressable
          className="flex flex-row my-2 items-center "
          style={{
            backgroundColor: index === hoveredIndex ? '#404249' : '#2B2D31',
            cursor: 'pointer',
          }}
          onPress={() => {
            store.dispatch.appState.updateUI({
              selected_guild_id: '',
              selected_channel_id: item.id,
              selected_tab: 'FRIENDS',
              selected_channel_name: `@${user.username}`,
            });
          }}
          onHoverIn={event => {
            setHoveredIndex(index);
          }}
          onHoverOut={() => {
            setHoveredIndex(null);
          }}>
          <UserIcon user={user} width={32} height={32} />
          <Text className="text-white ml-2 font-semibold">{user.username}</Text>
        </Pressable>
      );
    } else {
      return null;
    }
  };

  return (
    <FlatList
      data={private_channels}
      renderItem={renderItem}
      keyExtractor={dm => dm.id}
      contentContainerStyle={{ padding: 8 }}
      showsVerticalScrollIndicator={false}
      ListHeaderComponent={() => {
        return (
          <View className="ml-1 my-2">
            <Text className="text-[#949BA4] text-xs font-medium">
              DIRECT MESSAGES
            </Text>
          </View>
        );
      }}
    />
  );
};

export default Friends;

Expected Behavior

No response

Actual Behavior

No response

Reproducible Demo

No response

Additional context

No response

@neo773 neo773 added the bug Something isn't working label Jun 24, 2023
@Saadnajmi
Copy link
Collaborator

I've personally found that Flatlist is quite slow on macOS and haven't completely root caused it quite yet. I wonder if the issue is more Flatlist than the actual prop. You might want to try just putting a bunch of views in a ScrollView to compare, or using an alternative like Flashlist.

@neo773
Copy link
Author

neo773 commented Jun 24, 2023

I've personally found that Flatlist is quite slow on macOS and haven't completely root caused it quite yet. I wonder if the issue is more Flatlist than the actual prop. You might want to try just putting a bunch of views in a ScrollView to compare, or using an alternative like Flashlist.

I've also found it slow but my current issue is with hover events it's just super slow in general.
Rendering it in a normal list didn't make difference either.
It's reproducible even with a single element try moving the cursor in and out quickly over it.

@Saadnajmi
Copy link
Collaborator

Side note, @neo773 are you a discord engineer? Or building a discord client using public APIs?

@Saadnajmi
Copy link
Collaborator

In a similar demo of a with lots of items that override onHoverIn and onHoverOut (which you can find here), I'm not able to repro the error. I may have a faster machine as well, but it feels like there's something inherently different between the two videos more than just hardware. From the attached video, it feels more like a symptom of extraneous re-renders or re-rendering the whole View instead of just the item for every hover. Could you try making a blank or simple app and reproducing, and/or try using something like why-did-you-render to verify the only re-rendered element on hover is the Pressable?

Screen.Recording.2023-06-25.at.12.14.31.AM.mov

@neo773
Copy link
Author

neo773 commented Jun 25, 2023

So I looked at the test you referenced and it seems like it's rendering a native menu list I could not find any onHoverIn or onHoverOut use in the test code, I even went deeper into the component's code and didn't find any reference there either.

Anyways here's a minimal reproducible code

import { Text, FlatList } from 'react-native';
import React, { useState } from 'react';
import { Pressable, View } from 'react-native-macos';

const BugScreen = () => {
  const [hoveredIndex, setHoveredIndex] = useState<number | null>(null);

  const dummyData = Array.from({ length: 100 }, (_, index) => ({
    id: `dm-${index}`,
    user: {
      username: `User ${index}`,
    },
  }));

  const renderItem = ({
    item: item,
    index: index,
  }: {
    item: {
      id: string;
      user: {
        username: string;
      };
    };
    index: number;
  }) => {
    return (
      <Pressable
        style={{
          flexDirection: 'row',
          marginVertical: 2,
          alignItems: 'center',
          backgroundColor: index === hoveredIndex ? '#404249' : 'transparent',
          // @ts-expect-error
          cursor: 'pointer',
        }}
        onHoverIn={event => {
          setHoveredIndex(index);
        }}
        onHoverOut={() => {
          setHoveredIndex(null);
        }}>
        <Text style={{ color: 'black', marginLeft: 2, fontWeight: 'bold' }}>
          {item.user.username}
        </Text>
      </Pressable>
    );
  };

  return (
    <FlatList
      data={dummyData}
      renderItem={renderItem}
      keyExtractor={dm => dm.id}
      contentContainerStyle={{ padding: 8 }}
      showsVerticalScrollIndicator={false}
      ListHeaderComponent={() => {
        return (
          <View style={{ marginLeft: 1, marginVertical: 2 }}>
            <Text style={{ color: '#949BA4', fontSize: 12, fontWeight: '500' }}>
              DIRECT MESSAGES
            </Text>
          </View>
        );
      }}
    />
  );
};

export default BugScreen;

Side note, @neo773 are you a discord engineer? Or building a discord client using public APIs?

Nah, I'm building an open source client using private APIs

@Saadnajmi
Copy link
Collaborator

@neo773 not a native menu, a React Native drop in replacement for a menu, which each item being JS (I worked on it 🙂). 'useMenuItem' would be where where hover is used, but the code is convoluted, so don't feel the need to look deeper.

I'll look at your example code when I can!

@Saadnajmi
Copy link
Collaborator

@neo773 not a native menu, a React Native drop in replacement for a menu, which each item being JS (I worked on it 🙂). 'useMenuItem' would be where where hover is used, but the code is convoluted, so don't feel the need to look deeper.

I'll look at your example code when I can!

I'll try running your example code when I can. However just from looking at it, the useState for the hook is top level, and will re-render the whole Flatlist every time. Try using "listItemComponent' instead of renderItem (not well documented, but it's basically renderItem with better support for hooks), and see if you can perhaps wrap the item in a call to React.useMemo() to prevent unnecessary renders?

It's a busy week for me, but I'll try running your code when I can too.

@neo773
Copy link
Author

neo773 commented Jun 25, 2023

Tried the ListItemComponent prop and memoizing the item, now it just skips lot of frames

It's a busy week for me, but I'll try running your code when I can too.

I get it, take your time and thanks for looking into it.

new.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs: Triage 🔍
Projects
None yet
Development

No branches or pull requests

2 participants