-
Notifications
You must be signed in to change notification settings - Fork 127
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
Support for inline code blocks #695
Comments
@NickGerleman What do you think about this? I took much effort to sketch this idea in a clear way, also ensuring it fits well into the existing React Native ecosystem. I hope this proposal can be considered serious and achievable. I'm tagging you because we already had an opportunity to work on the React Native |
I think a prop like Right now, these are flattened into a single stream of rich text characters drawn by the platform's underlying text system. On Android, you do get a RN does allow views inline in text as well. This is akin to an element formatted with |
I managed to prototype this feature on both iOS and Android. It's a proof-of-concept of respective platforms' possibilities, in tiny Swift/Kotlin native apps. Both platforms resisted, but it was possible to achieve the desired effect on both of them in a surprisingly analogous manner. To support left/right padding, it was necessary to inject artificial characters into the To support spanning across multiple lines, on both platforms, it was necessary to override some new (from React Native's perspective) APIs. On iOS it was necessary to subclass @NickGerleman What's the next step here? Should I file a PR with a formal API proposal? Should I file a separate one for each new feature, or one joint one, or maybe only |
Please feel free to make PRs for these, preferably as granular as possible. I have context to review the Android implementation, and can try to flag someone to look at the iOS one. |
@NickGerleman I was asking about feature proposal PRs in this repo (React Native: Discussions and Proposals), did you also mean that? So far this is an issue. |
The first PR is slowly getting closer to being merged (🎉), but it's not the end of our quest. It's just the beginning. I want to start organizing the next steps. First topic I would like to raise is modelling the new styles on the "core" ( Currently, we have As I see it, what's necessary is to adjust it to a list of spans, which would hold styles like background color, border and padding. The rest of text styles could be kept in fragments. A span would contain a list of fragments. This assumes we don't want to support span nesting, at least initially. What do you think about this? Is it a reasonable model? |
Summary: A first step in my work on react-native-community/discussions-and-proposals#695 De-duplicate the code for creating `Spannable` on Android. I'm planning to add quite serious new features to this module. This would be really hard with the current level of code duplication. ## Changelog: [INTERNAL] [CHANGED] - De-duplicate building `Spannable` on Android Pull Request resolved: #39630 Test Plan: I tried to ensure that the refactored code is relatively easy to prove to be equivalent to the original duplicated one, but there's always a risk of a human mistake in this process. So far, I have been testing this by ensuring that nothing broke in the `Text` example section in RNTester. Reviewed By: mdvacca Differential Revision: D51016244 Pulled By: NickGerleman fbshipit-source-id: e9f873c01b2af0685c7b0943aebea170c997d22e
@NickGerleman @cortinico I opened a few small PRs, trying (maybe naively) to ask Meta engineers who (somewhat) recently worked on the surrounding code. I hoped it could somehow parallelize the process. This is mostly minor refactoring which makes the diff of the later changes more reviewer-friendly. I would hope to merge this in less than a month. |
Also, could I get my own issue in the React Native repo for tracking this project? |
Sure feel free to open an issue an I can assign it to you |
This comment was marked as outdated.
This comment was marked as outdated.
Summary: Extract fragment conversions to separate functions to make refactoring easier and simplify reasoning about the code. This code is being modified later. This is a minor improvement in the context of my multi-PR work on react-native-community/discussions-and-proposals#695. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGE] - Extract fragment conversions to separate functions Pull Request resolved: #42597 Reviewed By: NickGerleman Differential Revision: D52960655 Pulled By: robhogan fbshipit-source-id: 0df62b9980c06a1c2fc113d645ba8b6b668fa394
Summary: Clean up the function naming in `TextMeasureCache.h`. One name was clearly a human mistake. Make the naming consistent. This is a minor improvement in the context of my multi-PR work on react-native-community/discussions-and-proposals#695. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGE] - Clean up the function naming in `TextMeasureCache.h` Pull Request resolved: #42598 Reviewed By: NickGerleman Differential Revision: D52960435 Pulled By: sammy-SC fbshipit-source-id: 01327610446933972e8dc87e1b6e2950b7c706d2
Summary: Increase the readability of `CustomLineHeightSpan` by making the logic less stateful. This is a minor improvement in the context of my multi-PR work on react-native-community/discussions-and-proposals#695. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGED] - Increase the readability of `CustomLineHeightSpan` Pull Request resolved: #42592 Test Plan: - Prove the equivalence of the old and the new logic - Test that the behavior of `lineHeight` doesn't change Reviewed By: NickGerleman Differential Revision: D53028467 Pulled By: mdvacca fbshipit-source-id: d533bb77c8e10c29d8f2acc8cc39565d0013b03b
) Summary: `RCTAttributedTextUtils.mm`: Split `NSAttributedString` creation to functions in preparation for adding new logic here. This is a minor improvement in the context of my multi-PR work on react-native-community/discussions-and-proposals#695. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGE] - Refactor `NSAttributedString` creation in `RCTAttributedTextUtils.mm` Pull Request resolved: #42595 Reviewed By: cipolleschi Differential Revision: D53001495 Pulled By: sammy-SC fbshipit-source-id: 52d28e48f0a9d88d44325a73c64737fc7ac97781
Summary: Move all `ReactSpan` subclasses to a separate folder. This is a minor improvement in the context of my multi-PR work on react-native-community/discussions-and-proposals#695. I'm adding a new span class later, which was the direct motivation for this change. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGE] - Move all `ReactSpan` subclasses to a separate folder Pull Request resolved: #42594 Reviewed By: mdvacca Differential Revision: D53123733 Pulled By: cortinico fbshipit-source-id: 10db214a520d157c231e6f3b97948b4209a7ad4b
Summary: De-duplicate the logic for counting attachments. This is a minor improvement in the context of my multi-PR work on react-native-community/discussions-and-proposals#695. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [CHANGE] - De-duplicate the logic for counting attachments Pull Request resolved: #42596 Reviewed By: rshest Differential Revision: D53917281 Pulled By: cipolleschi fbshipit-source-id: cdb9bc834bddd7deffc60f33578464733982fedf
Introduction
Multiple existing apps (both native and web apps) have support for so-called inline code blocks. Just a few examples:
Slack
Discord
GitHub
Lorem
ipsum dolor
lorem ipsumInline code blocks can span across multiple lines if necessary.
Details
It's difficult or impossible to directly achieve the inline code block effect in React Native.
As I see it, there are three things in React Native that block us from achieving the discussed effect easily.
The line gap
Unlike CSS on the Web, if the inline text has a background color set, it fills the whole line. In CSS, it tightly wraps the inline text.
CSS:
React Native:
What's interesting, it seems there's no straightforward way to achieve the first effect on the second platform and vice-versa.
Such a feature exists in another W3 standard, TTML, and is named
fillLineGap
. It was added in 2017.Also in 2017, there was some discussion about adding such a feature to CSS.
It doesn't seem like much was done on this topic after that.
CSS behaves as if it had implicit
fill-line-gap: false
, and React Native behaves as if it hadfill-line-gap: true
.Inline padding
In CSS, inline elements can have padding.
In React Native, the
padding
style property is ignored for inline text. As the current behavior is to fill the whole line height, it's not clear how padding should behave (or if it should be honored at all).Borders
In CSS, inline elements can have borders, also with rounded corners.
The exact behavior is controlled by the box-decoration-break property.
In React Native, border styles for inline text are ignored.
Discussion points
I'd like to discuss what's necessary to support inline code blocks in React Native.
fillLineGap
style property to React Native reasonable?true
(fill the whole gap), for backward compatibilityfalse
value would mean CSS-like behaviorfillLineGap
, do we see any blockers from implementing these two inline text styles:What do you think?
The text was updated successfully, but these errors were encountered: