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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cherry Pick] Only apply isHighlighted native prop on iOS #1892

Merged
merged 1 commit into from
Jul 28, 2023

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
17 of 18 checks passed
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

3 participants