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

Migrate Icon/index.js and FloatingActionButton to function component #26654

Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
8b2eca6
Migrate Icon/index.js to function component
rayane-djouah Aug 18, 2023
16a2adf
fix lint errors
rayane-djouah Aug 18, 2023
c41e8f2
fix lint errors
rayane-djouah Aug 18, 2023
ebf0236
fix errors
rayane-djouah Aug 18, 2023
3ad83ef
declare 'displayName' outside the class body
rayane-djouah Aug 18, 2023
ac21c39
run prettier
rayane-djouah Aug 18, 2023
9b50cb8
fix for the fab plus icon, migrate FloatingActionButton to function c…
rayane-djouah Sep 4, 2023
062b7a6
Delete .gitconfig
rayane-djouah Sep 4, 2023
08ac9fe
refactor FloatingActionButton ref use, and renam the file to index.js
rayane-djouah Sep 4, 2023
4f20a47
rename FabPlusIcon component
rayane-djouah Sep 4, 2023
5cb09cc
rename FabPlusIcon component
rayane-djouah Sep 4, 2023
c5186bd
Merge branch 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Sep 9, 2023
9fe1125
run prettier
rayane-djouah Sep 9, 2023
a3c6977
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Sep 11, 2023
43373cf
remove unused proptypes and add missing ones
rayane-djouah Sep 15, 2023
91482f4
update react-native-svg
rayane-djouah Sep 21, 2023
32b2162
Merge branch 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Sep 21, 2023
8e060c3
fix Typescript error
rayane-djouah Sep 21, 2023
8eb99b1
revert latest change
rayane-djouah Sep 23, 2023
96b5d82
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Sep 23, 2023
a8406e0
remove forwarding ref
rayane-djouah Sep 23, 2023
a5a253a
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Sep 25, 2023
c055e06
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Sep 25, 2023
330e50e
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Sep 27, 2023
b5db485
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Sep 28, 2023
b88da06
Fix (On iOS, the + button is showing as a square): Preserve borderRad…
rayane-djouah Sep 28, 2023
1ed2d44
add easing to FabPlusIcon
rayane-djouah Oct 3, 2023
2caca58
Merge 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Oct 8, 2023
6647b25
bugfix/react-native-reanimated: Fix animated UI Props on Web #5169
rayane-djouah Oct 8, 2023
31939a4
delete react-native-reanimated patch
rayane-djouah Oct 8, 2023
0795952
fix size
rayane-djouah Oct 8, 2023
6281aa2
move Fab files to components dir
rayane-djouah Nov 29, 2023
90bfde0
Merge branch 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Nov 30, 2023
5921a2c
move FAB files to FAB dir
rayane-djouah Nov 30, 2023
5d5ab51
update react-native-reanimated to 3.6.0
rayane-djouah Nov 30, 2023
8c9750f
Merge branch 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Nov 30, 2023
67751ef
fix lint errors
rayane-djouah Nov 30, 2023
1fd316c
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Nov 30, 2023
630c3f6
rename back accessibility role prop
rayane-djouah Nov 30, 2023
bc09b8d
update react-native-reanimated to version 3.6.1
rayane-djouah Nov 30, 2023
0b881d4
pod update for react-native-reanimated 3.6.1
rayane-djouah Dec 5, 2023
43adece
Merge branch 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Dec 9, 2023
d0c7daa
Merge branch 'main' into Migrate-Icon/index.js-to-function-component
rayane-djouah Dec 9, 2023
8964dc9
fix floatingActionButton style use
rayane-djouah Dec 9, 2023
416229d
fix the default props usage and use hooks instead of HOC
rayane-djouah Dec 15, 2023
3648e1b
replace WithTheme and WithThemeStyles HOC with useTheme and useThemeS…
rayane-djouah Dec 15, 2023
b42f9f0
Merge branch 'Expensify:main' into Migrate-Icon/index.js-to-function-…
rayane-djouah Dec 16, 2023
a6da78c
fix lint error
rayane-djouah Dec 16, 2023
5bc120d
fix failing test
rayane-djouah Dec 16, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 3 additions & 28 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -777,35 +777,10 @@ PODS:
- React-Core
- RNReactNativeHapticFeedback (1.14.0):
- React-Core
- RNReanimated (3.5.4):
- DoubleConversion
- FBLazyVector
- glog
- hermes-engine
- RCT-Folly
- RCTRequired
- RCTTypeSafety
- React-callinvoker
- RNReanimated (3.6.1):
- RCT-Folly (= 2021.07.22.00)
- React-Core
- React-Core/DevSupport
- React-Core/RCTWebSocket
- React-CoreModules
- React-cxxreact
- React-hermes
- React-jsi
- React-jsiexecutor
- React-jsinspector
- React-RCTActionSheet
- React-RCTAnimation
- React-RCTAppDelegate
- React-RCTBlob
- React-RCTImage
- React-RCTLinking
- React-RCTNetwork
- React-RCTSettings
- React-RCTText
- ReactCommon/turbomodule/core
- Yoga
- RNScreens (3.21.0):
- React-Core
- React-RCTImage
Expand Down Expand Up @@ -1280,7 +1255,7 @@ SPEC CHECKSUMS:
rnmapbox-maps: 6f638ec002aa6e906a6f766d69cd45f968d98e64
RNPermissions: 9b086c8f05b2e2faa587fdc31f4c5ab4509728aa
RNReactNativeHapticFeedback: 1e3efeca9628ff9876ee7cdd9edec1b336913f8c
RNReanimated: ab2e96c6d5591c3dfbb38a464f54c8d17fb34a87
RNReanimated: fdbaa9c964bbab7fac50c90862b6cc5f041679b9
RNScreens: d037903436160a4b039d32606668350d2a808806
RNSVG: d00c8f91c3cbf6d476451313a18f04d220d4f396
SDWebImage: a7f831e1a65eb5e285e3fb046a23fcfbf08e696d
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
"react-native-plaid-link-sdk": "10.8.0",
"react-native-qrcode-svg": "^6.2.0",
"react-native-quick-sqlite": "^8.0.0-beta.2",
"react-native-reanimated": "3.5.4",
"react-native-reanimated": "^3.6.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Getting crash on android on latest main

android crash

Copy link
Member

Choose a reason for hiding this comment

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

weird. did you try npm ci? and rebuilding Android

Copy link
Contributor

Choose a reason for hiding this comment

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

I removed node_modules, installed npm and did fresh build

Copy link
Contributor

Choose a reason for hiding this comment

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

This also might be related

another

Copy link
Contributor

@tomekzaw tomekzaw Dec 17, 2023

Choose a reason for hiding this comment

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

@situchan Did you remember to rebuild the native app? Does npm start -- --reset-cache help?

Copy link
Contributor

Choose a reason for hiding this comment

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

Already did everything. I had to revert changes on this PR locally to unblock testing other PRs

Copy link
Contributor

@tomekzaw tomekzaw Dec 17, 2023

Choose a reason for hiding this comment

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

You're right, I can indeed reproduce this issue on 912fa3f as well. Working on it!

edit: the issue also appears if I downgrade Reanimated from 3.6.1 to 3.5.4. The error comes from fill property in FabPlusIcon.js.

Copy link
Member

Choose a reason for hiding this comment

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

@tomekzaw if possible could you please help us understand why the error comes from fill?
Is the API being used incorrectly?

https://github.com/Expensify/App/pull/26654/files#diff-58517876b1c001c5b56c9c446d55dd75c290c4e07c766fdbcb32e0d90d0d42f7R26

Copy link
Contributor

@tomekzaw tomekzaw Dec 18, 2023

Choose a reason for hiding this comment

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

@rushatgabhane I'm back with answers. Fortunately, it's not a regression in Reanimated 3.6.1 because on 3.5.4 it behaves the same way. It's also not an issue with react-native-svg (13.14.0 is okay).

If you animate fill prop of SVG elements, you need to pass adapter to useAnimatedProps, see the docs:
https://docs.swmansion.com/react-native-reanimated/docs/core/useAnimatedProps/#adapters-

You need the following adapter:

  const adapter = createAnimatedPropAdapter(
    (props) => {
      // delete props.fill;
      if (Object.keys(props).includes('fill')) {
        props.fill = { type: 0, payload: processColor(props.fill) };
      }
    },
    ['fill']
  );

Make sure to import processColor from react-native-reanimated (not react-native). Then, you pass the adapter as third argument to useAnimatedProps:

  const animatedProps = useAnimatedProps(
    () => {
      return {
        fill: interpolateColor(animatedValue.value, [0, 1], ['red', 'blue']),
      };
    },
    [],
    adapter
  );

Here's the full example for the context:

import React, { useEffect } from 'react';
import Animated, {
  Easing,
  createAnimatedPropAdapter,
  interpolateColor,
  useAnimatedProps,
  useSharedValue,
  processColor,
  withTiming,
} from 'react-native-reanimated';
import Svg, { Path } from 'react-native-svg';

const AnimatedPath = Animated.createAnimatedComponent(Path);

function FabPlusIcon() {
  const animatedValue = useSharedValue(0);

  useEffect(() => {
    animatedValue.value = withTiming(1, {
      duration: 340,
      easing: Easing.inOut(Easing.ease),
    });
  }, [animatedValue]);

  const adapter = createAnimatedPropAdapter(
    (props) => {
      // delete props.fill;
      if (Object.keys(props).includes('fill')) {
        props.fill = { type: 0, payload: processColor(props.fill) };
      }
    },
    ['fill']
  );

  const animatedProps = useAnimatedProps(
    () => {
      return {
        fill: interpolateColor(animatedValue.value, [0, 1], ['red', 'blue']),
      };
    },
    [],
    adapter
  );

  return (
    <Svg width={20} height={20}>
      <AnimatedPath
        d="M12,3c0-1.1-0.9-2-2-2C8.9,1,8,1.9,8,3v5H3c-1.1,0-2,0.9-2,2c0,1.1,0.9,2,2,2h5v5c0,1.1,0.9,2,2,2c1.1,0,2-0.9,2-2v-5h5c1.1,0,2-0.9,2-2c0-1.1-0.9-2-2-2h-5V3z"
        animatedProps={animatedProps}
      />
    </Svg>
  );
}

FabPlusIcon.displayName = 'FabPlusIcon';

export default FabPlusIcon;

cc @WoLewicki, would be good to provide some abstraction for it, e.g. useAnimatedSVGProps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think we should also add stroke there so it looks something like this:

  const adapter = createAnimatedPropAdapter(
    (props) => {
      if (Object.keys(props).includes('fill')) {
        props.fill = { type: 0, payload: processColor(props.fill) };
      }
      if (Object.keys(props).includes('stroke')) {
        props.stroke = {type: 0, payload: processColor(props.stroke)}
      }
    },
    ['fill', 'stroke']
  );

useAnimatedSVGProps sounds good 🚀

"react-native-render-html": "6.3.1",
"react-native-safe-area-context": "4.4.1",
"react-native-screens": "3.21.0",
Expand Down
132 changes: 0 additions & 132 deletions src/components/FloatingActionButton.js

This file was deleted.

48 changes: 48 additions & 0 deletions src/components/FloatingActionButton/FabPlusIcon.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import PropTypes from 'prop-types';
import React, {useEffect} from 'react';
import Animated, {Easing, interpolateColor, useAnimatedProps, useSharedValue, withTiming} from 'react-native-reanimated';
import Svg, {Path} from 'react-native-svg';
import themeColors from '@styles/themes/default';

const AnimatedPath = Animated.createAnimatedComponent(Path);

const propTypes = {
/* Current state (active or not active) of the component */
isActive: PropTypes.bool.isRequired,
};

function FabPlusIcon({isActive}) {
const animatedValue = useSharedValue(isActive ? 1 : 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

const sharedValue


useEffect(() => {
animatedValue.value = withTiming(isActive ? 1 : 0, {
duration: 340,
easing: Easing.inOut(Easing.ease),
});
}, [isActive, animatedValue]);

const animatedProps = useAnimatedProps(() => {
const fill = interpolateColor(animatedValue.value, [0, 1], [themeColors.textLight, themeColors.textDark]);

return {
fill,
};
});

return (
<Svg
width={20}
height={20}
>
<AnimatedPath
d="M12,3c0-1.1-0.9-2-2-2C8.9,1,8,1.9,8,3v5H3c-1.1,0-2,0.9-2,2c0,1.1,0.9,2,2,2h5v5c0,1.1,0.9,2,2,2c1.1,0,2-0.9,2-2v-5h5c1.1,0,2-0.9,2-2c0-1.1-0.9-2-2-2h-5V3z"
animatedProps={animatedProps}
rayane-djouah marked this conversation as resolved.
Show resolved Hide resolved
/>
</Svg>
);
}

FabPlusIcon.propTypes = propTypes;
FabPlusIcon.displayName = 'FabPlusIcon';

export default FabPlusIcon;
87 changes: 87 additions & 0 deletions src/components/FloatingActionButton/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import PropTypes from 'prop-types';
import React, {useEffect, useRef} from 'react';
import {View} from 'react-native';
import Animated, {Easing, interpolateColor, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import PressableWithFeedback from '@components/Pressable/PressableWithFeedback';
import Tooltip from '@components/Tooltip/PopoverAnchorTooltip';
import withTheme, {withThemePropTypes} from '@components/withTheme';
import withThemeStyles, {withThemeStylesPropTypes} from '@components/withThemeStyles';
import useLocalize from '@hooks/useLocalize';
import compose from '@libs/compose';
import FabPlusIcon from './FabPlusIcon';

const AnimatedPressable = Animated.createAnimatedComponent(PressableWithFeedback);
AnimatedPressable.displayName = 'AnimatedPressable';

const propTypes = {
/* Callback to fire on request to toggle the FloatingActionButton */
onPress: PropTypes.func.isRequired,
rayane-djouah marked this conversation as resolved.
Show resolved Hide resolved

/* Current state (active or not active) of the component */
isActive: PropTypes.bool.isRequired,

/* An accessibility label for the button */
accessibilityLabel: PropTypes.string.isRequired,

/* An accessibility role for the button */
role: PropTypes.string.isRequired,

...withThemeStylesPropTypes,
...withThemePropTypes,
};

const FloatingActionButton = React.forwardRef(({onPress, isActive, accessibilityLabel, role, theme, themeStyles}, ref) => {
const {translate} = useLocalize();
const fabPressable = useRef(null);
const animatedValue = useSharedValue(isActive ? 1 : 0);
const buttonRef = ref;

useEffect(() => {
animatedValue.value = withTiming(isActive ? 1 : 0, {
duration: 340,
easing: Easing.inOut(Easing.ease),
});
}, [isActive, animatedValue]);

const animatedStyle = useAnimatedStyle(() => {
const backgroundColor = interpolateColor(animatedValue.value, [0, 1], [theme.success, theme.buttonDefaultBG]);

return {
transform: [{rotate: `${animatedValue.value * 135}deg`}],
backgroundColor,
borderRadius: themeStyles.floatingActionButton.borderRadius,
};
});

return (
<Tooltip text={translate('common.new')}>
<View style={themeStyles.floatingActionButtonContainer}>
<AnimatedPressable
ref={(el) => {
fabPressable.current = el;
if (buttonRef) {
buttonRef.current = el;
}
}}
accessibilityLabel={accessibilityLabel}
role={role}
pressDimmingValue={1}
onPress={(e) => {
// Drop focus to avoid blue focus ring.
fabPressable.current.blur();
onPress(e);
}}
onLongPress={() => {}}
style={[themeStyles.floatingActionButton, animatedStyle]}
>
<FabPlusIcon isActive={isActive} />
</AnimatedPressable>
</View>
</Tooltip>
);
});

FloatingActionButton.propTypes = propTypes;
FloatingActionButton.displayName = 'FloatingActionButton';

export default compose(withThemeStyles, withTheme)(FloatingActionButton);
Loading
Loading