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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(iOS): line break mode prop iOS updates to consume new cpp functions #46129

Conversation

shubhamguptadream11
Copy link
Collaborator

@shubhamguptadream11 shubhamguptadream11 commented Aug 21, 2024

Summary:

Solves this issue: #44107

Changelog:

[IOS] [ADDED] - Line break mode for TextInput components. This includes iOS updates to consume new cpp functions.

This PR is a breakdown of this PR.

Test Plan:

  • Tested builds in new and old architecture mode.

@facebook-github-bot 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 Aug 21, 2024
@react-native-bot
Copy link
Collaborator

Fails
🚫

📋 Verify Changelog Format - See Changelog format

Generated by 🚫 dangerJS against 397a0be

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Aug 21, 2024
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Thanks for working on this and for splitting the original PR

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@cipolleschi
Copy link
Contributor

hey @shubhamguptadream11, I was testing the new changes and I'm not sure they work as intended...
I think that the clip should clip inside the TextInput without ellipsis, not to go over the textInput boundaries... 😅

Also, can we use word everywhere instead of wordWrapping?

@cipolleschi
Copy link
Contributor

the issue can be due to a recent change we pushed that refactors how we draw the text in RCTParagraphComponentView. Try to rebase on top of main to see it failing.

@shubhamguptadream11
Copy link
Collaborator Author

shubhamguptadream11 commented Aug 23, 2024

hey @shubhamguptadream11, I was testing the new changes and I'm not sure they work as intended...
I think that the clip should clip inside the TextInput without ellipsis, not to go over the textInput boundaries... 😅

I verified this behaviour by testing NSLineBreakByClipping in a sample iOS app (not in React Native).
I observed that when setting NSLineBreakByClipping in paragraphStyle.lineBreakMode, the text indeed overflows the text field area instead of clipping it inside.
I've attached the sample code and a snapshot demonstrating this.

    // Create a paragraph style
    NSMutableParagraphStyle *paragraphStyle = [[NSMutableParagraphStyle alloc] init];

    paragraphStyle.lineBreakMode = NSLineBreakByClipping; // Example line break mode
    
    // Create attributes dictionary
    NSDictionary<NSAttributedStringKey, id> *attributes = @{
        NSParagraphStyleAttributeName: paragraphStyle,
        NSFontAttributeName: [UIFont systemFontOfSize:16], // Example font size
        NSForegroundColorAttributeName: [UIColor blackColor] // Example text color
    };
    
    // Create attributed string with attributes
    NSAttributedString *attributedText = [[NSAttributedString alloc] initWithString:self.textField.text attributes:attributes];
    
    // Set attributed text to the text field
    self.textField.attributedText = attributedText;
    
    // Set up border
    self.textField.layer.borderWidth = 1.0; // Border width
    self.textField.layer.borderColor = [UIColor grayColor].CGColor; // Border color
    self.textField.layer.cornerRadius = 5.0; // Optional: for rounded corners
    
    // Add the text field to the view
    [self.view addSubview:self.textField];

simulator_screenshot_291E3C86-EE1C-483F-8F81-E13A8902858B

Solution:
In order to achieve the desired clipping effect within the boundaries in react-native, adding overflow: 'hidden' is necessary. So we can update the comments stating that in order to use clip use overflow: 'hidden' as well.

Also, can we use word everywhere instead of wordWrapping?

I am renaming the wordWrapping to word

Thanks!

@cipolleschi
Copy link
Contributor

That's an interesting behavior. xD
It does not respect what it states in the documentation. 😞
Screenshot 2024-08-23 at 16 06 17

Also, I have not realized that wordWrapping was already used in the old arch... so let's try not to be breaking.
But don't worry, I can take care of it.

@shubhamguptadream11
Copy link
Collaborator Author

@cipolleschi I already pushed the changes related to renaming. So do you want me to revert it? or you can handle it.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 23, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in fe941a8.

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. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants