microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.28k stars 1.14k forks source link

Expose props for setting TextLineBounds and LineStackingStrategy #9218

Closed aschultz closed 1 year ago

aschultz commented 2 years ago

Summary

XAML has two properties which control the bounding region and layout of each line of text: TextLineBounds and LineStackingStrategy.

Both of these properties should be exposed as React props on the <Text /> element to enable fine-tuning of text layout, such as when vertically centering text within a button. The default XAML values are extremely problematic.

Motivation

By default, XAML uses a TextLineBounds of Full, meaning that the bounding box includes ascenders and descenders. The height of ascenders/descenders relative to the font baseline can vary by font, and a run of text may or may not have ascenders or descenders. The result is that the position of the baseline relative to the bounding box can vary significantly and this makes it impossible to consistently positioning the line of text relative to other elements. Vertically positioning text within a button, for example, is an exercise in frustration as any change to the text or font can shift it up or down.

This article goes into detail about the same problem on web, which currently lacks the knobs XAML provides. It describes an active proposal to add new CSS properties like leading-trim to address the problem.

XAML has this functionality already available (set TextLineBounds="Tight"), we just need to expose it through React.

As for LineStackingStrategy... it normally defaults to MaxHeight, but RNW recently (as of 0.65) changed this to BlockLineHeight because of MaxHeight's poor handling of small line heights. This change has the side-effect of making multi-line text much more consistent (it will simply obey line height instead of shifting around based on the content). However, there may be cases where the other available values (MaxHeight and BaselineToBaseline) are useful. I suggest exposing it as well.

Basic Example

No response

Open Questions

Should this be exposed as an RNW-specific prop or should it align with the CSS leading-trim spec?

chrisglein commented 2 years ago

If there's a gap here RN-wide then we'd want to make sure whatever solution we come up with is communicated out so we can have alignment across RN platforms, even if we do something for Windows first.

chrisglein commented 2 years ago

@aschultz Do you have any workarounds you're employing in the meantime to get your text measurement exact? How blocked are you? Coming up with a solution here that aligns all of RN will take more time.

aschultz commented 2 years ago

AFAIK, it's not possible to work around this aside from patching the Text component to support the property. We have cases where devs manually added padding or margins to position text correctly, but they're extremely fragile and specific to each usage of a control.

Looking at RN's iOS code, I think an implementation is possible since they actually have full access to the font metrics and do the line height / offset calculations directly. Not sure about Android.

Related - There's an outstanding bug on RN that looks similar to this fix in RNW. So anyone diving into this on the RN side could probably knock both out at the same time and bring some consistency across platforms.

rozele commented 2 years ago

It's theoretically possible to do this outside of the core RN Text component using refs and findNodeHandle, but generally speaking findNodeHandle isn't supported very well (at all?) in Fabric. We use this pattern quite a bit where we do something like...

const textRef = useRef<Text>();
useEffect(() => {
  // call native module to do something to native component
  if (textRef.current) {
    MyCustomNativeModule.setExtraTextProperty(findNodeHandle(textRef.current));
  }

  return () => {
    // call native module to cleanup what was done to native component
   MyCustomNativeModule.cleanupExtraTextProperty(findNodeHandle(textRef.current));
  }
});
...
return (
  <Text ref={textRef} ...>...</Text>
)

For example, we've done this to build a custom native module that extends the ContextFlyout for TextBlock and TextBox ( and ) with extra options and also to add JS events for handling TextBox.Paste native events.

It's not ideal to do something like this for trimming / stacking strategy as you'd want these properties to be correct on initial rendering (to avoid re-rendering the text), but it could work.

In general, RN lacks APIs to extend the prop set / events for core components, however at least on iOS and Android, you theoretically have the capability to derive from core view managers and either replace the core implementations that include an extended property set or at least create another view manager that does not need to copy the code from the core version.

rozele commented 2 years ago

That being said, it could be a good contribution to core, but I'm not sure how well it will work with the custom measure functions for text on iOS and Android (sounds like you might have already done some digging on iOS).