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

[Web] Adjust findNodeHandle to properly detect SVG #3197

Merged
merged 7 commits into from
Nov 7, 2024
Merged

Conversation

m-bert
Copy link
Contributor

@m-bert m-bert commented Nov 4, 2024

Description

#3127 introduced our own implementation of findNodeHandle on web. Unfortunately, it doesn't work if someone uses GestureDetector on SVG elements, e.g.:

<Svg width={200} height={200}>
  <GestureDetector gesture={tapGestureCircle}>
    <Circle r={200} cx={210} cy={210} fill={circleFill} />
  </GestureDetector>
</Svg>

Code above would render additional div element with display: 'contents';, which would break SVG.

This PR does the following things:

  1. Bumps react-native-svg version
  2. Modifies Wrap component such that now if element is part of SVG it doesn't wrap it into additional div
  3. Adjusts findNodeHandle function to properly handle SVG refs

Test plan

Tested on the following example:
import {
  GestureHandlerRootView,
  GestureDetector,
  Gesture,
} from 'react-native-gesture-handler';
import { View } from 'react-native';
import { Svg, Circle } from 'react-native-svg';
import { useState, useCallback } from 'react';

export default function App() {
  const [circleFill, setCircleFill] = useState('blue');
  const switchCircleColor = useCallback(
    () => setCircleFill((old) => (old === 'blue' ? 'brown' : 'blue')),
    [setCircleFill]
  );

  const tapGestureCircle = Gesture.Tap().runOnJS(true).onEnd(switchCircleColor);

  return (
    <GestureHandlerRootView style={{ flex: 1, paddingTop: 200 }}>
      <View style={{ padding: 10, borderWidth: 1, alignSelf: 'flex-start' }}>
        <Svg width={200} height={200}>
          <GestureDetector gesture={tapGestureCircle}>
            <Circle r={200} cx={210} cy={210} fill={circleFill} />
          </GestureDetector>
        </Svg>
      </View>
    </GestureHandlerRootView>
  );
}

@m-bert m-bert requested review from j-piasecki and jakex7 November 4, 2024 11:36
Copy link
Member

@j-piasecki j-piasecki left a comment

Choose a reason for hiding this comment

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

(whispers into the void) Who's gonna tell him about MathMLElement?

src/web/utils.ts Outdated Show resolved Hide resolved
src/findNodeHandle.web.ts Outdated Show resolved Hide resolved
@m-bert m-bert requested review from jakex7 and j-piasecki November 5, 2024 09:55
src/web/utils.ts Outdated Show resolved Hide resolved
src/web/utils.ts Outdated Show resolved Hide resolved
@m-bert m-bert requested a review from jakex7 November 7, 2024 10:35
Copy link
Member

@jakex7 jakex7 left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@m-bert m-bert merged commit ef686fc into main Nov 7, 2024
3 checks passed
@m-bert m-bert deleted the @mbert/fix-svg-web branch November 7, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants