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

Text component does not respect border properties #5187

Closed
kmelmon opened this issue Jun 12, 2020 · 4 comments · Fixed by #5740
Closed

Text component does not respect border properties #5187

kmelmon opened this issue Jun 12, 2020 · 4 comments · Fixed by #5740

Comments

@kmelmon
Copy link
Contributor

kmelmon commented Jun 12, 2020

The border properties should apply to all components in react-native but the RNW code only applies them to certain components:
-View
-Image
-Components based on ContentControl

Any other component, like Text, does not respect the border properties.

See: https://snack.expo.io/kJlEGti8g

On other platforms, notice the border draws around the text. For example this is what the snack looks like on Android:

Capture_Android

On Windows it looks like this:

Capture_Windows

Oops! No border is rendered.

@kmelmon kmelmon added the bug label Jun 12, 2020
@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Jun 12, 2020
@kmelmon
Copy link
Contributor Author

kmelmon commented Jun 12, 2020

This looks to be a general problem that will likely affect any non-ContentControl-based component.

@kmelmon kmelmon changed the title Text component (and others) does not respect border properties Text component does not respect border properties Jun 12, 2020
@chrisglein chrisglein added API: Completion Area: Borders and Brushes Area: Text and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Jun 15, 2020
@chrisglein chrisglein added this to the 0.63 (M6) milestone Jun 15, 2020
@chrisglein
Copy link
Member

We know that Facebook has issues with some components not having borders that they expected. I'm still waiting to hear back if this is one of the ones they're waiting on. If so, we'll want to prioritize for 0.63. If not, this is still a good thing to fill in for API parity but less pressing.

@rectified95
Copy link
Contributor

@kmelmon Is this perhaps fixed by #5213 ?

@kmelmon
Copy link
Contributor Author

kmelmon commented Aug 5, 2020

Unfortunately #5213 won't fix this. The problem here is that Text maps to TextBlock and Run, which doesn't have border properties.

If we consider the full problem it gets pretty tricky because Text can be nested. In the nested case we have a Run that can wrap so the border isn't guaranteed to be a simple rectangle in that case.

I suggest we prioritize fixing the simple case first, which is to handle the border properties only on the outer TextBlock. I can think of several ways to fix this:

  1. Add border properties to the XAML TextBlock. This means waiting for WinUI 3.0.
  2. Wrap the TextBlock with an outer Control, similar to what we do for View when it has certain properties set. This is rather complicated but seems doable. Wondering out loud: Could we perhaps wrap the Text component with an outer View on the JS side and forward all the properties except the border-related ones to the inner Text? This might be a simpler solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants