-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[iOS][TextInput]Add numberOfLines and maxNumberOfLines props to TextInput on iOS #41510
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Janic Duplessis <janicduplessis@gmail.com>
…acebook#37609) Summary: Pull Request resolved: facebook#37609 changelog: [internal] Moving LayoutMetrics and LayoutPrimitives from core to graphics module. This is to enable different implementation for different platforms. Reviewed By: rubennorte Differential Revision: D45904748 fbshipit-source-id: a4e666d7c7390e87abdb09235f96655b63f451f9
…o graphics folder Differential Revision: D45904748 Original commit changeset: a4e666d7c739 Original Phabricator Diff: D45904748 fbshipit-source-id: c2da28836cb51966854c81d4e380a2abeb742cda
Summary: Chronos Job Instance ID: 1125907889493757 Sandcastle Job Instance ID: 36028797978627335 allow-large-files ignore-conflict-markers opt-out-review Differential Revision: D46270525 fbshipit-source-id: 26beec108aa03c970338fdc4561af24798813acf
Summary: Pull Request resolved: facebook#37572 Currently, RNTester was using a completely custom AppDelegate and not leveraging the RCTAppDelegate we use in the OSS. This resulted in a misalignment between the two setups and duplicated work to test stuff internally furst and then in the OSS, with some more time needed to understand why one setup was working and the other wasn't. With this change, we are aligning the two, bringing RNTester closer to the OSS setup. There are still small differences, but we can iterate over those. ## Changelog: [iOS][Changed] - Make RNTester use RCTAppDelegate Reviewed By: cortinico Differential Revision: D46182888 fbshipit-source-id: 7c55b06de1a317b1f2d4ad0d18a390dc4d3356a4
Summary: Pull Request resolved: facebook#37596 This commit is a backport of this [commit](facebook@32327cc) so that we build the right version of hermes in CI. This also bumps the hermes keys to make sure that we are building it properly from now on. ## Changelog: [Internal] - Fix Hermes versioning and bump hermes workspace keys Reviewed By: cortinico Differential Revision: D46225179 fbshipit-source-id: f00c9d20d37f3bd5689e0742063f98e3bfe373c2
Summary: Pull Request resolved: facebook#37614 changelog: [internal] We do not control what `vImageBoxConvolve_ARGB8888` returns, it may return 0. If it does return 0, we will allocate memory chunk of size 0. Yes, malloc will let you do that. Well, it depends on the implementation, but according to the spec it is legal. The only requirement is to by able to call free on that without crash. If `vImageBoxConvolve_ARGB8888` does return 0 and we allocate memory of size 0. Call to `vImageBoxConvolve_ARGB8888` with tempBuffer of size 0 will lead to a crash. [The documentation](https://developer.apple.com/documentation/accelerate/1515945-vimageboxconvolve_argb8888#discussion) for `vImageBoxConvolve_ARGB8888` and tempBuffer states: > To determine the minimum size for the temporary buffer, the first time you call this function pass the kvImageGetTempBufferSize flag. Pass the same values for all other parameters that you intend to use in for the second call. The function returns the required minimum size, which **should be a positive value**. (A negative returned value indicates an error.) The kvImageGetTempBufferSize flag prevents the function from performing any processing other than to determine the minimum buffer size. I think the keyword word there is "should be a positive value". 0 is not a positive value. Reviewed By: javache, yungsters Differential Revision: D46263204 fbshipit-source-id: baa8fac5b3be6fb5bed02800cd725cc4cf43485a
…targets (facebook#37581) Summary: In Xcode projects with multiple targets, and in particular when targets are for different platforms (e.g. iOS and macOS), Cocoapods may add a suffix to a Pod name like `React-Core`. When this happens, the code in `new_architecture.rb` (which was looking for a pod with exact name `React-Core`) would not add the preprocessor definitions for Fabric as expected. This change fixes this issue. Fixes facebook#37102 . ## Changelog: [iOS] [Fixed] - Fix Fabric issue with React-Core pod when Xcode project has multiple targets Pull Request resolved: facebook#37581 Test Plan: Tested that this change fixes this issue which occurs 100% of the time in React Native TV projects. Reviewed By: dmytrorykun Differential Revision: D46264704 Pulled By: cipolleschi fbshipit-source-id: 8dfc8e342b5a110ef1f028636e01e5c5f2b6e2f0
Summary: This (`greet.yml`) action requires some _changes_ to "repo" settings which are _out of maintainers' controls_. So, instead of wasting compute; let's just delete this action :( 🤗🌏 ## Changelog: [GENERAL] [REMOVED] - [Actions] Remove `greet.yml` action Pull Request resolved: facebook#37587 Test Plan: - Not needed. Reviewed By: cortinico Differential Revision: D46275607 Pulled By: cipolleschi fbshipit-source-id: 80880568cbae1158006445e078e638e4e375cb73
Summary: Pull Request resolved: facebook#37616 Add tests in CircleCI to avoid that changes in Hermes can break the Xcode integration. ## Changelog: [Internal] - Add CI tests to avoid to break the Hermes-Xcode integration Reviewed By: cortinico Differential Revision: D46265656 fbshipit-source-id: 176f1a33fe6944a68fb14b62e5c626fe30d296cc
Summary: Pull Request resolved: facebook#37410 Incremental adoption of new bridgeless API's, where they are semantically equivalent to the old one. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D45736529 fbshipit-source-id: e41f6840e7c329f6051530e53773fae760584842
facebook-github-bot
added
the
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
Nov 16, 2023
Closing this PR since this will most probably not be accepted into the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This pull-request adds
rows
,numberOfLines
props to TextInput component on iOS so that it we can use it on all platforms. Additionally, the addsmaxNumberOfLines
prop that lets people express maximumHeight in lines (which is useful due to multithreaded react native architecture that can cause some delays caused by scheduling)The main idea in this pr is to make use of
maximumNumberOfLines
property onNSTextContainer
. As we don't have any native support for setting exact height expressed in lines I came up with the idea that we can add some lines of text at the layout stage what will let us implementnumberOfLines
usingmaximumNumberOfLines
. The lines are not added in component that are actually displaying text so that it's not visible on the screen but is using on layout layer soShadowNodes
andTextLayoutManagers
. Later similar idea was implemented on Android here (#35703) but I split the implementation to simplify the review.So you can review this pr without a need to get familiar with original pr.
Changelog:
[iOS] [Added] - Added support for numberOfLines/rows to TextInput on iOS platform (Fabric and Paper)
[iOS] [Added] - Added MaxNumberOfLines prop to the same component that sets upper bound instead of exact value
Test Plan:
I added few Texts to RN tester