callstack / react-native-paper

Material Design for React Native (Android & iOS)
https://reactnativepaper.com
MIT License
12.83k stars 2.08k forks source link

FAB Group Icons rotating when using an IconSource #2067

Closed salindersidhu closed 4 years ago

salindersidhu commented 4 years ago

Current behaviour

Hey, I've been using FAB.Group with IconSources and noticed an issue with the most recent version of react-native-paper. This was working in version 3.10.1. Using Fab.Group with action FABs that use IconSource causes the icons of the action FABs to rotate when opening and closing.

This works correctly when providing an icon name rather than an IconSource. I've provided code samples and GIFs of both working and problematic cases.

Expected behaviour

The icons of the action FABs of FAB.Group should not rotate when using an IconSource.

Code sample

Working

import React from 'react';
import {SafeAreaView} from 'react-native';

import {Provider, Portal, FAB} from 'react-native-paper';

const App = () => {
  const [state, setState] = React.useState({open: false});

  const {open} = state;
  const onStateChange = ({open}) => setState({open});

  return (
    <SafeAreaView style={{flex: 1}}>
      <Provider>
        <Portal>
          <FAB.Group
            open={open}
            icon={open ? 'close' : 'menu'}
            onStateChange={onStateChange}
            actions={[
              {
                icon: 'heart',
                onPress: () => console.log('Pressed heart'),
              },
              {icon: 'bell', onPress: () => console.log('Pressed bell')},
              {icon: 'star', onPress: () => console.log('Pressed star')},
              {
                icon: 'crosshairs-gps',
                onPress: () => console.log('Pressed GPS'),
              },
            ]}
          />
        </Portal>
      </Provider>
    </SafeAreaView>
  );
};

export default App;

Problematic

import React from 'react';
import {SafeAreaView} from 'react-native';

import {Provider, Portal, FAB} from 'react-native-paper';
import IonIcons from 'react-native-vector-icons/Ionicons';
import MaterialIcons from 'react-native-vector-icons/MaterialIcons';

const App = () => {
  const [state, setState] = React.useState({open: false});

  const {open} = state;
  const onStateChange = ({open}) => setState({open});

  return (
    <SafeAreaView style={{flex: 1}}>
      <Provider>
        <Portal>
          <FAB.Group
            open={open}
            icon={open ? 'close' : 'menu'}
            onStateChange={onStateChange}
            actions={[
              {
                icon: (props) => <IonIcons {...props} name="heart" />,
                onPress: () => console.log('Pressed heart'),
              },
              {
                icon: (props) => <IonIcons {...props} name="layers" />,
                onPress: () => console.log('Pressed layers'),
              },
              {
                icon: (props) => <IonIcons {...props} name="star" />,
                onPress: () => console.log('Pressed star'),
              },
              {
                icon: (props) => <MaterialIcons {...props} name="gps-fixed" />,
                onPress: () => console.log('Pressed GPS'),
              },
            ]}
          />
        </Portal>
      </Provider>
    </SafeAreaView>
  );
};

export default App;

Screenshots (if applicable)

Working

Problematic

What have you tried

I had a similar issue with the icon inside Appbar.Action but I was able to fix that by using the animation prop and setting that to false. This doesn't work for FAB.Group FABs.

Your Environment

software version
ios or android Both
react-native 0.63.1
react-native-paper 4.0.1
react-native-vector-icons 7.0.0
node 12.16.1
npm 6.13.6
github-actions[bot] commented 4 years ago

Couldn't find version numbers for the following packages in the issue:

Can you update the issue to include version numbers for those packages? The version numbers must match the format 1.2.3.

The versions mentioned in the issue for the following packages differ from the latest versions on npm:

Can you verify that the issue still exists after upgrading to the latest versions of these packages?

kamui545 commented 4 years ago

Might be related to https://github.com/callstack/react-native-paper/issues/2069

salindersidhu commented 4 years ago

I did some digging and found that this change is what broke it. https://github.com/callstack/react-native-paper/pull/1896/files

It fixes the original issue of not having a FAB with a custom Icon Source animate but the fix is also affecting the child FABs in the FAB Group. The fix for this might involve adding an animated prop to FAB.

bboure commented 4 years ago

I am facing this issue too.

The reason is that each time the component is re-rendered, the icon function is re-defined, causing the CrossFadeIcon component to think it has changed (isEqualIcon returns false)

For now the solution is to define the function outside the render() method (for Class components), or outside the main function (for functional Components).

This is a bit annoying though, especially if you don't need animation.

A better solution would be indeed to have an animated property, like in IconButton that you could set to false If animation is required, then this should be handled properly by the developer as described above.

satya164 commented 4 years ago

@bboure yea, we should add animated property explicitly. Can you send a PR? :)

bboure commented 4 years ago

Hi all, I opened a PR that "fixes" this, but as I explain in the description, I am not satisfied about it. This is open for discussion. See #2100

Meanwhile, I have come with a solution to solve this problem, even in the current version. I was already using a helper that generates the function and returns it for me. So what I did is that I memoize the returned functions to maintain equality. That seems to work pretty well.

Example:

const getIconKey = (icon: IconProp): string => {
  return typeof icon === 'string'
    ? ['fas', icon].toString()
    : Array.isArray(icon)
    ? icon.toString()
    : [icon.prefix, icon.iconName].toString()
};

export const fontAwesome = (icon: IconProp): IconSource => {
  const memoKey = getIconKey(icon);

  if (!faMemo[memoKey]) {
    faMemo[memoKey] = ({ size, color }) => {
      return (<FontAwesomeIcon icon={icon} size={size} color={color} />);
    };
  }

  return faMemo[memoKey];
};

In my case, I am using FontAwesome, functions are memoized by icon Prefix and Name.

Usage:

<FAB icon={fontAwesome(['far', 'filter'])} />