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

Remove TextInput setting insertionPointColor #1321

Merged
merged 1 commit into from
Aug 5, 2022

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

We should not be setting the insertionPointColor by default.
Let the OS do its thing.
Later we can consider exposing a prop for this but the default should be to not set it.

Changelog

[macOS] [Changed] - Remove TextInput setting insertionPointColor

Test Plan

Setting a purple selectionColor on the texInput fields

selectionColor='purple'

Before

before

before2

After

after

after2

@christophpurrer christophpurrer requested a review from a team as a code owner August 2, 2022 23:49
We should not be setting the insertionPointColor by default.
Let the OS do its thing. Later we can consider exposing a prop for this but the default should be to not set it.
@christophpurrer
Copy link
Author

@Saadnajmi > Is yarn test-ios broken in the main branch?

I just ran it locally and I get the same error as in the failed ci/circleci: test_ios_unit_hermes job

...
Test Suite 'RNTesterUnitTests.xctest' passed at 2022-08-04 10:13:37.177.
	 Executed 172 tests, with 0 failures (0 unexpected) in 0.454 (0.543) seconds
Test Suite 'Selected tests' passed at 2022-08-04 10:13:37.178.
	 Executed 172 tests, with 0 failures (0 unexpected) in 0.454 (0.545) seconds
2022-08-04 10:13:37.457 xcodebuild[39674:12232662] [MT] IDETestOperationsObserverDebug: 115.546 elapsed -- Testing started completed.
2022-08-04 10:13:37.457 xcodebuild[39674:12232662] [MT] IDETestOperationsObserverDebug: 0.000 sec, +0.000 sec -- start
2022-08-04 10:13:37.457 xcodebuild[39674:12232662] [MT] IDETestOperationsObserverDebug: 115.546 sec, +115.546 sec -- end

Test session results, code coverage, and logs:
	/Users/chpurrer/Library/Developer/Xcode/DerivedData/RNTesterPods-fluhyrocwhnsapbnumzfxhrxwalw/Logs/Test/Run-RNTester-2022.08.04_10-10-41--0700.xcresult

** TEST SUCCEEDED **

error Command failed with signal "SIGTERM".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Can't start packager automatic

I have the same problem in #1322

@chiuam chiuam merged commit 0f665ba into microsoft:main Aug 5, 2022
christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Aug 9, 2022
# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m

# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m
christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Aug 9, 2022
# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m

# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m
christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Aug 10, 2022
# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m

# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
# Conflicts:
#	Libraries/Text/TextInput/Multiline/RCTUITextView.m
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants