fix: datatable.title numberOfLines > 1 height + wrapped text align

[mikehardy]

Summary

Thanks for react-native-paper you all, it's fantastic.

The DataTable.Title component takes a numberOfLines prop

However, the internal / wrapped Text component has a style applied with height and lineHeight fixed at 24.
So it appears that even with numberOfLines > 1, the height of the Text will be fixed and not allow multiple lines.

So without this, it does not appear the numberOfLines prop has an effect, as noticed in #848 and #863

With this, it appears to work exactly as expected.

Text components require special alignment treatment with word wrapping. Normally a word wrap will force text alignment to flex-start. So we align directly when number of lines is > 1, while respecting the numeric property and it's interaction with RTL languages

It's entirely possible I'm missing something, there was one success report in comments of #848, and the PR author must have made it work, but I just could not see how to make it work without this? And neither could others.

Test plan

I went right into node_modules and made the edits to test it. I also ran it in the example app and updated the jest snapshots. The jest tests show that my change is backwards-compatible - it has no effect when numberOfLines === 1 other than to move where height prop is set

I'm using patch-package to use this change right now.

[mikehardy]

my testing was inadequate at first - there's a problem with the scope of the variable, also when text wraps it wrecks the center alignment of the text, repairing that needs to take into account whether it's numeric or not... more complete solution in a bit

[callstack-bot]

Hey @mikehardy, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

[mikehardy]

react-native-paper+4.11.1.patch.txt

In case anyone else wants to test this, here's a patch-package patch for it

[mikehardy]

With space, variety of column headings, emphasizing 1 vs 2 number of lines and numeric or not:

Starting to smash the table in, you can see the numeric behavior and the wrapping behavior working on numeric and non-numeric

Before this all the headers just clipped with one line

[mikehardy]

@lukewalczak 👋 happy new year! Is there anything I can do to help on this one to try and get it merged? I thought it was a bit subtle because of the RTL but I did my best on it and I think I got it right. Cheers

[lukewalczak]

Hello @mikehardy, I will do my best to check your PR this week.

[lukewalczak]

@mikehardy Would you mind please attaching the code snippet (or snack) which I can compare then with your new changes to observe the difference like on the screenshots?

[mikehardy]

Yeah - give me a little bit to do it - like a couple days - as a reviewer I would want that as well, my screenshots are "interesting" but not the concrete proof you can touch like a snack.

[lukewalczak]

Yeah - give me a little bit to do it - like a couple days -

Sure, take your time

as a reviewer I would want that as well, my screenshots are "interesting" but not the concrete proof you can touch like a snack.

Awesome, thank you!

[mikehardy]

This isn't dead - I still have this patch integrated and it's on my list to get a good reproduction here, I've just been buried. Still loving react-native-paper in my projects, either way ;-)

[mikehardy]

Sorry for the long delay
Here's the snack showing that number of lines is simply not respected with the current version, plus a demo of the compiled version with the PR integrated:

https://snack.expo.dev/jkMkOqCKg

Reproduction code is:

import * as React from 'react';
import { DataTable } from 'react-native-paper';

const MyComponent = () => {

  const widerCell = {
    minWidth: 150,
    width: 130,
    justifyContent: 'center',
    alignItems: 'center',
  };

  const borderCell = {
    borderStartWidth: 1,
    justifyContent: 'center',
    alignItems: 'center',
  };

  return (
            <DataTable>
              <DataTable.Header>
                <DataTable.Title>Fund</DataTable.Title>
                <DataTable.Title style={widerCell}>
                  Credit Type
                </DataTable.Title>
                <DataTable.Title numberOfLines={2} style={borderCell}>
                  Allow Reservation
                </DataTable.Title>
                <DataTable.Title numberOfLines={2} style={borderCell}>
                  Allow Fee Offset
                </DataTable.Title>
                <DataTable.Title numberOfLines={2} style={borderCell}>
                  Capital Dividend
                </DataTable.Title>
                <DataTable.Title numberOfLines={2} style={borderCell}>
                  External Exchange
                </DataTable.Title>
                <DataTable.Title numberOfLines={2} style={borderCell}>
                  Fund Exchange
                </DataTable.Title>
              </DataTable.Header>
            <DataTable.Row>
              <DataTable.Cell>No credit types found.</DataTable.Cell>
            </DataTable.Row>
        </DataTable>
  );
}

export default MyComponent;

You can also see several things going wrong at once if you try it - here's a screen cap showing it in the second-to-last column header which is wrapped at this width - when wrapping happens in the current implementation it loses the alignment for reasons explained in the commit messages. It's really subtle to get this one correct!

[mikehardy]

@lukewalczak if you're releasing tomorrow it would be great to get this one in as well, done my best to give you a repro and demonstrate not just the existing behavior but also the fix so you can see it in action

[lukewalczak]

Hey @mikehardy, thank you for your commitment to fixing the problem and demonstrating the repro. Changes LGTM 🎉

[mikehardy]

Likewise @lukewalczak - thanks for react-native-paper, I'm really enjoying using it in my projects!