From 46cc7031bcb9f5d8f0f2437b64291720c2328e72 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Fri, 17 Dec 2021 12:30:14 -0500 Subject: [PATCH 1/2] fix: datatable.title cell height multiple of numberOfLines 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 --- src/components/DataTable/DataTableTitle.tsx | 23 ++++++++++++++++- .../__snapshots__/DataTable.test.js.snap | 25 +++++++++++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/components/DataTable/DataTableTitle.tsx b/src/components/DataTable/DataTableTitle.tsx index 7c0dee2598..8ebd8ddc0e 100644 --- a/src/components/DataTable/DataTableTitle.tsx +++ b/src/components/DataTable/DataTableTitle.tsx @@ -128,6 +128,16 @@ const DataTableTitle = ({ 1 + ? numeric + ? I18nManager.isRTL + ? styles.leftText + : styles.rightText + : styles.centerText + : {}, sortDirection ? styles.sorted : { color: textColor }, textStyle, ]} @@ -150,12 +160,23 @@ const styles = StyleSheet.create({ paddingVertical: 12, }, + rightText: { + textAlign: 'right', + }, + + leftText: { + textAlign: 'left', + }, + + centerText: { + textAlign: 'center', + }, + right: { justifyContent: 'flex-end', }, cell: { - height: 24, lineHeight: 24, fontSize: 12, fontWeight: '500', diff --git a/src/components/__tests__/__snapshots__/DataTable.test.js.snap b/src/components/__tests__/__snapshots__/DataTable.test.js.snap index 9029cc84fe..581e235823 100644 --- a/src/components/__tests__/__snapshots__/DataTable.test.js.snap +++ b/src/components/__tests__/__snapshots__/DataTable.test.js.snap @@ -104,9 +104,12 @@ exports[`renders data table header 1`] = ` "alignItems": "center", "fontSize": 12, "fontWeight": "500", - "height": 24, "lineHeight": 24, }, + Object { + "height": 24, + }, + Object {}, Object { "color": "rgba(0, 0, 0, 0.6)", }, @@ -158,9 +161,12 @@ exports[`renders data table header 1`] = ` "alignItems": "center", "fontSize": 12, "fontWeight": "500", - "height": 24, "lineHeight": 24, }, + Object { + "height": 24, + }, + Object {}, Object { "color": "rgba(0, 0, 0, 0.6)", }, @@ -1472,9 +1478,12 @@ exports[`renders data table title with press handler 1`] = ` "alignItems": "center", "fontSize": 12, "fontWeight": "500", - "height": 24, "lineHeight": 24, }, + Object { + "height": 24, + }, + Object {}, Object { "marginLeft": 8, }, @@ -1562,9 +1571,12 @@ exports[`renders data table title with sort icon 1`] = ` "alignItems": "center", "fontSize": 12, "fontWeight": "500", - "height": 24, "lineHeight": 24, }, + Object { + "height": 24, + }, + Object {}, Object { "marginLeft": 8, }, @@ -1670,9 +1682,12 @@ exports[`renders right aligned data table title 1`] = ` "alignItems": "center", "fontSize": 12, "fontWeight": "500", - "height": 24, "lineHeight": 24, }, + Object { + "height": 24, + }, + Object {}, Object { "color": "rgba(0, 0, 0, 0.6)", }, From f1e8ec03788a78c9871692d98bee132ef6296287 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Fri, 17 Dec 2021 14:05:17 -0500 Subject: [PATCH 2/2] fix: set maxHeight as multiple of numberOfLines, not height If you set height, it will be numberOfLines high even if there is enough room such that wrapping is not needed. But with lineHeight set to 24 and forcing the minimum height with no wrapping, you just need maxHeight set to hold all the lines when wrapped --- src/components/DataTable/DataTableTitle.tsx | 2 +- .../__tests__/__snapshots__/DataTable.test.js.snap | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/DataTable/DataTableTitle.tsx b/src/components/DataTable/DataTableTitle.tsx index 8ebd8ddc0e..54ae387123 100644 --- a/src/components/DataTable/DataTableTitle.tsx +++ b/src/components/DataTable/DataTableTitle.tsx @@ -129,7 +129,7 @@ const DataTableTitle = ({ style={[ styles.cell, // height must scale with numberOfLines - { height: 24 * numberOfLines }, + { maxHeight: 24 * numberOfLines }, // if numberOfLines causes wrap, center is lost. Align directly, sensitive to numeric and RTL numberOfLines > 1 ? numeric diff --git a/src/components/__tests__/__snapshots__/DataTable.test.js.snap b/src/components/__tests__/__snapshots__/DataTable.test.js.snap index 581e235823..4e77fe0c73 100644 --- a/src/components/__tests__/__snapshots__/DataTable.test.js.snap +++ b/src/components/__tests__/__snapshots__/DataTable.test.js.snap @@ -107,7 +107,7 @@ exports[`renders data table header 1`] = ` "lineHeight": 24, }, Object { - "height": 24, + "maxHeight": 24, }, Object {}, Object { @@ -164,7 +164,7 @@ exports[`renders data table header 1`] = ` "lineHeight": 24, }, Object { - "height": 24, + "maxHeight": 24, }, Object {}, Object { @@ -1481,7 +1481,7 @@ exports[`renders data table title with press handler 1`] = ` "lineHeight": 24, }, Object { - "height": 24, + "maxHeight": 24, }, Object {}, Object { @@ -1574,7 +1574,7 @@ exports[`renders data table title with sort icon 1`] = ` "lineHeight": 24, }, Object { - "height": 24, + "maxHeight": 24, }, Object {}, Object { @@ -1685,7 +1685,7 @@ exports[`renders right aligned data table title 1`] = ` "lineHeight": 24, }, Object { - "height": 24, + "maxHeight": 24, }, Object {}, Object {