-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Changes from all commits
8b2eca6
16a2adf
c41e8f2
ebf0236
3ad83ef
ac21c39
9b50cb8
062b7a6
08ac9fe
4f20a47
5cb09cc
c5186bd
9fe1125
a3c6977
43373cf
91482f4
32b2162
8e060c3
8eb99b1
96b5d82
a8406e0
a5a253a
c055e06
330e50e
b5db485
b88da06
1ed2d44
2caca58
6647b25
31939a4
0795952
6281aa2
90bfde0
5921a2c
5d5ab51
8c9750f
67751ef
1fd316c
630c3f6
bc09b8d
0b881d4
43adece
d0c7daa
8964dc9
416229d
3648e1b
b42f9f0
a6da78c
5bc120d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
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 useTheme from '@hooks/useTheme'; | ||
|
||
const AnimatedPath = Animated.createAnimatedComponent(Path); | ||
|
||
const propTypes = { | ||
/* Current state (active or not active) of the component */ | ||
isActive: PropTypes.bool.isRequired, | ||
}; | ||
|
||
function FabPlusIcon({isActive}) { | ||
const theme = useTheme(); | ||
const animatedValue = useSharedValue(isActive ? 1 : 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
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], [theme.textLight, theme.textDark]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI if you use // this is regular React code
const { baz } = foo.bar;
useAnimatedProps(() => {
// this is a worklet
// use `baz` directly
}); |
||
|
||
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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
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 useLocalize from '@hooks/useLocalize'; | ||
import useTheme from '@hooks/useTheme'; | ||
import useThemeStyles from '@hooks/useThemeStyles'; | ||
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, | ||
}; | ||
|
||
const FloatingActionButton = React.forwardRef(({onPress, isActive, accessibilityLabel, role}, ref) => { | ||
const theme = useTheme(); | ||
const styles = useThemeStyles(); | ||
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: styles.floatingActionButton.borderRadius, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same story for |
||
}; | ||
}); | ||
|
||
return ( | ||
<Tooltip text={translate('common.new')}> | ||
<View style={styles.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={[styles.floatingActionButton, animatedStyle]} | ||
> | ||
<FabPlusIcon isActive={isActive} /> | ||
</AnimatedPressable> | ||
</View> | ||
</Tooltip> | ||
); | ||
}); | ||
|
||
FloatingActionButton.propTypes = propTypes; | ||
FloatingActionButton.displayName = 'FloatingActionButton'; | ||
|
||
export default FloatingActionButton; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 inFabPlusIcon.js
.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 passadapter
touseAnimatedProps
, see the docs:https://docs.swmansion.com/react-native-reanimated/docs/core/useAnimatedProps/#adapters-
You need the following adapter:
Make sure to import
processColor
fromreact-native-reanimated
(notreact-native
). Then, you pass the adapter as third argument touseAnimatedProps
:Here's the full example for the context:
cc @WoLewicki, would be good to provide some abstraction for it, e.g.
useAnimatedSVGProps
?There was a problem hiding this comment.
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:useAnimatedSVGProps
sounds good 🚀