Skip to content

fix hint animation #1363

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Jul 4, 2021
Merged

fix hint animation #1363

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4084a59
fix hint animation
mendyEdri Jun 14, 2021
876ca41
fix android error
mendyEdri Jun 20, 2021
a46173b
Merge branch 'master' into fix/hint-animation
mendyEdri Jun 20, 2021
721c588
Fix android tapping issue + remove hint on animation done
mendyEdri Jun 22, 2021
3fea5a5
typing
mendyEdri Jun 22, 2021
cd2cad2
lint
mendyEdri Jun 22, 2021
5663a13
Fix iOS behaviour
mendyEdri Jun 22, 2021
ec43535
typing
mendyEdri Jun 22, 2021
a066e45
Fix removal after animation timing
mendyEdri Jun 27, 2021
f8c51d3
removed log
mendyEdri Jul 4, 2021
84cbf2a
Using now a single toggle to remove Hint component when hidden
mendyEdri Jul 4, 2021
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
5 changes: 5 additions & 0 deletions generatedTypes/components/hint/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export interface HintProps {
interface HintState {
targetLayout?: HintTargetFrame;
targetLayoutInWindow?: HintTargetFrame;
animationEnded: boolean;
}
/**
* @description: Hint component for displaying a tooltip over wrapped component
Expand All @@ -122,12 +123,16 @@ declare class Hint extends Component<HintProps, HintState> {
static positions: typeof HintPositions;
targetRef: ElementRef<typeof RNView> | null;
hintRef: ElementRef<typeof RNView> | null;
animationDuration: number;
state: {
targetLayoutInWindow: undefined;
targetLayout: HintTargetFrame | undefined;
animationEnded: boolean;
};
visibleAnimated: Animated.Value;
componentDidUpdate(prevProps: HintProps): void;
animateHint: () => void;
toggleAnimationEndedToRemoveHint: () => void;
focusAccessibilityOnHint: () => void;
setTargetRef: (ref: ElementRef<typeof RNView>) => void;
setHintRef: (ref: ElementRef<typeof RNView>) => void;
Expand Down
52 changes: 34 additions & 18 deletions src/components/hint/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export interface HintProps {
interface HintState {
targetLayout?: HintTargetFrame;
targetLayoutInWindow?: HintTargetFrame;
animationEnded: boolean;
}

/**
Expand All @@ -165,24 +166,38 @@ class Hint extends Component<HintProps, HintState> {

targetRef: ElementRef<typeof RNView> | null = null;
hintRef: ElementRef<typeof RNView> | null = null;
animationDuration = 170;

state = {
targetLayoutInWindow: undefined,
targetLayout: this.props.targetFrame
targetLayout: this.props.targetFrame,
animationEnded: false
};

visibleAnimated = new Animated.Value(Number(!!this.props.visible));

componentDidUpdate(prevProps: HintProps) {
if (prevProps.visible !== this.props.visible) {
Animated.timing(this.visibleAnimated, {
toValue: Number(!!this.props.visible),
duration: 170,
useNativeDriver: true
}).start();
this.animateHint();
}
}

animateHint = () => {
Animated.timing(this.visibleAnimated, {
toValue: Number(!!this.props.visible),
duration: this.animationDuration,
useNativeDriver: true
}).start(this.toggleAnimationEndedToRemoveHint);
};

toggleAnimationEndedToRemoveHint = () => {
this.setState({animationEnded: true}, () => {
setTimeout(() => {
this.setState({animationEnded: false});
}, this.animationDuration);
});
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This forces us to trigger two setState calls every time we trigger the animation. twice on enter twice on exit.
Maybe we can reduce that..
If you think about it, while the Hint is visible it doesn't matter really what's the value of animationEnded state.

What if instead of keeping animationEnded you'll keep something else, like actuallyVisible, hasUnmounted (or anything with a better name (: )
Then you'll update it once on enter and once on exit...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, fixed


focusAccessibilityOnHint = () => {
const {message} = this.props;
const targetRefTag = findNodeHandle(this.targetRef);
Expand Down Expand Up @@ -444,13 +459,10 @@ class Hint extends Component<HintProps, HintState> {
renderHint() {
const {onPress, testID} = this.props;
const opacity = onPress ? 0.9 : 1.0;
const Container = onPress ? TouchableOpacity : View;


if (this.showHint) {
return (
<Container
activeOpacity={opacity}
onPress={onPress}
<View
animated
style={[
{width: this.containerWidth},
Expand All @@ -462,19 +474,23 @@ class Hint extends Component<HintProps, HintState> {
pointerEvents="box-none"
testID={testID}
>
<TouchableOpacity activeOpacity={opacity} onPress={onPress}>
{this.renderContent()}
</TouchableOpacity>
{this.renderHintTip()}
{this.renderContent()}
</Container>
</View>
);
}
}

renderHintContainer() {
const {style, testID, ...others} = this.props;
const {style, ...others} = this.props;
return (
<View
{...others}
testID={`${testID}.container`}
// this view must be collapsable, don't pass testID or backgroundColor etc'.
collapsable
testID={undefined}
style={[styles.container, style, this.getContainerPosition()]}
>
{this.renderHint()}
Expand All @@ -496,9 +512,9 @@ class Hint extends Component<HintProps, HintState> {
}

render() {
const {visible, onBackgroundPress, testID} = this.props;

if (!visible) {
const {onBackgroundPress, testID} = this.props;
if (!this.props.visible && this.state.animationEnded) {
console.log('removed');
Copy link
Collaborator

Choose a reason for hiding this comment

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

You forgot a console.log (:

return this.props.children;
}

Expand Down