Skip to content

Customize formatting styles #94

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

Merged
merged 32 commits into from
Jan 8, 2024
Merged

Customize formatting styles #94

merged 32 commits into from
Jan 8, 2024

Conversation

tomekzaw
Copy link
Collaborator

@tomekzaw tomekzaw commented Jan 3, 2024

Closes #20.

@tomekzaw tomekzaw requested a review from robertKozik January 5, 2024 14:22
Copy link
Collaborator

@robertKozik robertKozik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOW 😮 so great!

Comment on lines +15 to +16
- (void)textInputDidChange;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this? I don't see any change inside implementation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Fabric, [RCTTextInputComponentView textInputDidChange] is implemented in RCTTextInputComponentView.mm but there's no such declaration in RCTTextInputComponentView.

Since we want to call [_textInput textInputDidChange]; in the library code which imports the header, we need to add such declaration.

Without this line, the code won't compile due to an unknown selector.

paragraphStyle.headIndent = 5;
[attributedString addAttribute:NSParagraphStyleAttributeName value:paragraphStyle range:range];
[attributedString addAttribute:NSForegroundColorAttributeName value:_markdownStyle.preColor range:range];
// TODO: pass background color and ranges to layout manager
} else if ([type isEqualToString:@"h1"]) {
NSMutableParagraphStyle *paragraphStyle = [NSMutableParagraphStyle new];
NSRange range2 = NSMakeRange(range.location - 2, range.length + 2);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can change the name of this variable to be more descriptive? Or add comment to indicate that we need to wrap the syntax # as well as text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 74e9e75

Comment on lines +15 to +17
#ifdef RCT_NEW_ARCH_ENABLED
// implemented in MarkdownTextInputView updateProps:
#else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain me why we initialise markdown style in different place for fabric?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm got it, I guess, viewManager is paper only 😄

Copy link
Collaborator Author

@tomekzaw tomekzaw Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ViewManager is indeed a concept from the old architecture but it turns out that they are still required on the new architecture.

First, I've tried to wrap the contents of this file in #ifndef RCT_NEW_ARCH_ENABLED but it didn't work – the component was not properly registered. I think RCT_EXPORT_MODULE(MarkdownTextInputView) is responsible for registering custom views.

Then, I've tried to wrap RCT_CUSTOM_VIEW_PROPERTY(markdownStyle, NSDictionary, MarkdownTextInputViewView) and it also didn't work – the custom prop was not defined.

Finally, I decided to wrap only the implementation of the method; in fact this is necessary because initWithDictionary: is defined only on Paper. The Fabric counterpart is initWithStyle: which accepts a C++ struct generated by react-native-codegen. Since this struct is generated only on Fabric, it needs to be wrapped with #ifdef RCT_NEW_ARCH_ENABLED. That's why it makes sense to put the declaration of initWithStyle: in #else branch.

@tomekzaw tomekzaw force-pushed the @tomekzaw/customize-styles branch from e6163e6 to 4eccbcf Compare January 8, 2024 11:37
@tomekzaw tomekzaw force-pushed the @tomekzaw/customize-styles branch from 38fd8d0 to 890db3a Compare January 8, 2024 12:31
@tomekzaw tomekzaw merged commit 5837361 into main Jan 8, 2024
@tomekzaw tomekzaw deleted the @tomekzaw/customize-styles branch January 8, 2024 13:01
@tomekzaw tomekzaw mentioned this pull request Jan 8, 2024
tomekzaw added a commit that referenced this pull request Feb 5, 2024
@@ -48,8 +52,10 @@ protected void onAttachedToWindow() {

if (previousSibling instanceof ReactEditText) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E/App Issue Closure Checklist

The checklist asks to point to a possible offending PR regarding fixed issue:

which we fixed in PR #590 by adding applyNewStyles() at the end of this block.

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.

Customize formatting styles
3 participants