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

Allow scrollResponderZoomTo() on macOS #1265

Merged

Conversation

christophpurrer
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Already works on macOS (as it does on iOS).
Doesn't work on Android or Windows.

Changelog

[macOS] [Changed] - Allow scrollResponderZoomTo() on macOS

Test Plan

I have used the following sample in rn-tester in ScrollViewExample.js

const HorizontalScrollView = (props: {direction: 'ltr' | 'rtl'}) => {
  const {direction} = props;
  const scrollRef = React.useRef<?React.ElementRef<typeof ScrollView>>();
  const [scale, setScale] = useState(20);
  const [coord, setCoord] = useState(0);
  const title = direction === 'ltr' ? 'LTR Layout' : 'RTL Layout';
  return (
    <View style={{direction}}>
      <Text style={styles.text}>{title}</Text>
      <ScrollView
        ref={scrollRef}
        automaticallyAdjustContentInsets={false}
        horizontal={true}
        style={[styles.scrollView, styles.horizontalScrollView]}
        testID={'scroll_horizontal'}
      >
        {ITEMS.map(createItemRow)}
      </ScrollView>
      <Button
        label="Scroll to start"
        onPress={() => {
                setScale(scale + 20);
                setCoord(coord + 1);
                scrollRef.current?.scrollResponderZoomTo({
                  x: coord,
                  y: coord,
                  width: scale,
                  height: scale,
                  animated: true,
                });
          //nullthrows(scrollRef.current).scrollTo({x: 0});
        }}
        testID={'scroll_to_start_button'}
      />
      <Button
        label="Scroll to end"
        onPress={() => {
          nullthrows(scrollRef.current).scrollToEnd({animated: true});
        }}
        testID={'scroll_to_end_button'}
      />
    </View>
  );
};
Screen.Recording.2022-07-21.at.12.25.39.PM.mov

Also used at Meta for Messenger Desktop.

@christophpurrer christophpurrer requested a review from a team as a code owner July 21, 2022 19:36
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

Libraries/Components/ScrollView/ScrollView.js Outdated Show resolved Hide resolved
@ghost ghost removed the Needs: Author Feedback label Jul 21, 2022
@christophpurrer christophpurrer force-pushed the allowScrollResponderZoomTo branch 2 times, most recently from 8e3c70d to 54471f4 Compare July 21, 2022 21:51
@Saadnajmi Saadnajmi self-assigned this Jul 22, 2022
Libraries/Components/ScrollView/ScrollView.js Outdated Show resolved Hide resolved
Already works on macOS (as it does on iOS).
Doesn't work on Android or Windows
@harrieshin
Copy link

@Saadnajmi / @amgleitman : not sure why Apple PR failed with the following error. can you take a look, please?
RCTBundleURLProviderTests ✗ testBundleURL, ((URL) equal to (localhostBundleURL())) failed: ("file:///Applications/Xcode_13.3.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/Library/Xcode/Agents/main.jsbundle") is not equal to ("[http://localhost:8081/test.jsbundle.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=com.apple.dt.xctest.tool"](http://localhost:8081/test.jsbundle.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=com.apple.dt.xctest.tool%22)) ✗ testBundleURL, ((URL) equal to (localhostBundleURL())) failed: ("file:///Applications/Xcode_13.3.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/Library/Xcode/Agents/main.jsbundle") is not equal to ("[http://localhost:8081/test.jsbundle.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=com.apple.dt.xctest.tool"](http://localhost:8081/test.jsbundle.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=com.apple.dt.xctest.tool%22))

@Saadnajmi
Copy link
Collaborator

Saadnajmi commented Jul 22, 2022

@harrieshin intermittent failure. I reran and we're good now

@Saadnajmi Saadnajmi merged commit 90bb91a into microsoft:main Jul 22, 2022
@christophpurrer
Copy link
Author

Great!

@christophpurrer christophpurrer deleted the allowScrollResponderZoomTo branch July 22, 2022 19:33
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
Already works on macOS (as it does on iOS).
Doesn't work on Android or Windows

Co-authored-by: Scott Kyle <skyle@fb.com>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
Already works on macOS (as it does on iOS).
Doesn't work on Android or Windows

Co-authored-by: Scott Kyle <skyle@fb.com>
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Apr 27, 2023
Summary:
X-link: facebook/yoga#1265

This deprecates `YGConfigSetUseLegacyStretchBehaviour` and `YGConfigGetUseLegacyStretchBehaviour`and points users to errata APIs instead. Using the C API will fire deprecation warnings, which should create errors in builds with `-Werror`, though they can be ignored if truly needed (like we do with the language bindings which need to expose their own deprecated interface).

Differential Revision: D45337198

fbshipit-source-id: 8762d7f213c223605ade288febcaaa51e129cfa5
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.

None yet

5 participants