-
Notifications
You must be signed in to change notification settings - Fork 20
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
Substr measurements expensify #56
Substr measurements expensify #56
Conversation
@Szymon20000 could you have a look please? |
061b587
to
d6ef986
Compare
Instead of |
https://reactnative.dev/docs/text#nested-text
Nested text with style is essentially just a string and not a view component, therefore this implementation isn't straightforward to implement. |
Taking a look now. Wouldn't be cleaner to implement onLayout for nested Text components? We could then wrap fragment that has to be highlighted into nested text.
|
It would, however please take a look at the comment above.
|
It doesn't seems to be that hard. At least just checked it on iOS Fabric. |
BaseTextShadowNode::buildAttributedString <- This method creates fragments. It could also create regions |
Otherwise the code seems to be correct to me :) |
We could also add regionID prop for nested text instead. Then paragraphEventEmitter would return map {regionID: RegionMeasurment} |
@azimgd Any thoughts on @Szymon20000's comments? |
@azimgd Any help with the above issue? |
@parasharrajat this PR fix this issue |
Awesome. Thanks @perunt. I'll watch it. |
@azimgd thoughts on the feedback? |
I'm not entirely certain about the approach we should take here since it seems that the upstream PR will not be merged, and the expensify/RN fork is being abandoned. I'm actively working on it, but unfortunately cannot provide an estimate for when it will be completed. Let me know any thoughts or suggestions you may have, and I will keep you updated once I have smth. |
@azimgd We haven't finalized the decision of dropping the fork yet. If that is blocking you move forward, I will suggest we wait for the final verdict on it before you start spending time on a custom implementation. Also, it will be great to show us the plan before implementing it. |
I think the upstream pr could be merged. We just need to change the API a bit. |
Hey @azimgd. Is this still something you want to work on or shall we ask someone else to try one of the alternative approaches? |
Hey @puneetlath, I'm still exploring alternative approach. |
Ok cool. Can we help in any way? |
Let's go ahead and close this out since we're no longer taking this approach. Thanks for the effort everyone! |
Upstream PR Link
facebook#37397
Summary
I would like to apply styling to a substring of text, such as a background color, padding, and rounded corners. This functionality could be used for:
@username
is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen
book.The problem is that the Text element in RN behaves like a block element rather than an inline element when it is multiline. Additionally, it does not allow you to have borders or rounded corners. This makes it unsuitable for implementing a multiline inline code block.
Adding a new prop called textLayoutRegions which accepts an array of regions (substring positions) could be used to solve the problem. Passing this prop will calculate the regions [[start, end], [start, end] ...] position and pass the response into onTextLayout callback. Knowing exact substring position and dimensions will allow implementing custom styled View behind the text.
Changelog
[GENERAL] [ADDED] - New
textLayoutRegions
prop for Text component that accepts an array of substring regions. Each region's layout position will be appended intoonTextLayout
event callback.[GENERAL] [ADDED] - New
regions
props foronTextLayout
event callback.Test Plan