Skip to content

Conversation

@Saadnajmi
Copy link
Collaborator

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

During the 0.71 merge, I accidentally clobbered over some macOS specific diffs that were introduced here: cf48f8d#diff-8854d86a25d1052c7f65f4d7235198297464645255149b9700171beb00ca67ec

The solution is to cherry-pick a future change that should be more resistant to accidental-clobbering.

My notes from an internal thread about this:

I couldn't figure out why isHighlighted was iOS only so I added it back for macOS. I see that I even commented about that, my bad. I think the issue was that Shawn's orginal PR deleted the original iOS code, and we don't have anything like // [macOS] tags for deleted lines. Because there were no tags, when I compared diffs, I just added them back. I'm not sure how to avoid this in the future besides the usual "Don't modify the iOS code as much as possible" bits, or moving to Text.macos.js files like what was proposed earlier. For now, I think cherry picking that change to main / 0.71-stable makes sense. I can make those PRs later today or tomorrow if someone wasn't already.

Cherry-picked change notes:

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38642

isHighlighted is only used for iOS. Even macOS disables it (see https://github.com/microsoft/react-native-macos/pull/1346).

This change ensures that the isHighlighted prop is only updated for iOS.

Changelog:

[General] [Fixed] - Avoids re-renders during text selection on desktop platforms by limiting native-only isHighlighted prop to iOS

Reviewed By: lenaic, sammy-SC

Differential Revision: D47800845

fbshipit-source-id: af109be17027b2fbc9408e2ec9e1b841c709fe35


## Changelog

[MACOS] [FIXED] - Cherry pick a change to only apply `isHighlighted` on iOS


## Test Plan

CI passes

Summary:
Pull Request resolved: facebook#38642

isHighlighted is only used for iOS. Even macOS disables it (see microsoft#1346).

This change ensures that the isHighlighted prop is only updated for iOS.

## Changelog:

[General] [Fixed] - Avoids re-renders during text selection on desktop platforms by limiting native-only `isHighlighted` prop to iOS

Reviewed By: lenaic, sammy-SC

Differential Revision: D47800845

fbshipit-source-id: af109be17027b2fbc9408e2ec9e1b841c709fe35
@Saadnajmi Saadnajmi merged commit 8773dd0 into microsoft:main Jul 28, 2023
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