From ff51501b0c80667236bd11ec0c159605094c0577 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 7 Jan 2019 13:53:04 -0500 Subject: [PATCH] MM-11232 Fix table column width to better support wide tables (#2467) * MM-11232 Fix table column width to better support wide tables * Fix horizontal scrolling on Android --- app/components/markdown/markdown.js | 7 +- .../markdown/markdown_table/markdown_table.js | 81 ++++++++++++++----- .../markdown_table_cell.js | 12 ++- .../markdown_table_row/markdown_table_row.js | 11 ++- app/screens/table/table.js | 45 ++++++----- package-lock.json | 4 +- package.json | 2 +- 7 files changed, 114 insertions(+), 48 deletions(-) diff --git a/app/components/markdown/markdown.js b/app/components/markdown/markdown.js index ea79afc72..d14da9d2f 100644 --- a/app/components/markdown/markdown.js +++ b/app/components/markdown/markdown.js @@ -364,9 +364,12 @@ export default class Markdown extends PureComponent { return rendered; }; - renderTable = ({children}) => { + renderTable = ({children, numColumns}) => { return ( - + {children} ); diff --git a/app/components/markdown/markdown_table/markdown_table.js b/app/components/markdown/markdown_table/markdown_table.js index dbf9e0f74..2d6cf6d73 100644 --- a/app/components/markdown/markdown_table/markdown_table.js +++ b/app/components/markdown/markdown_table/markdown_table.js @@ -13,13 +13,15 @@ import LinearGradient from 'react-native-linear-gradient'; import {preventDoubleTap} from 'app/utils/tap'; import {changeOpacity, makeStyleSheetFromTheme} from 'app/utils/theme'; +import {CELL_WIDTH} from 'app/components/markdown/markdown_table_cell/markdown_table_cell'; -const MAX_HEIGHT = 120; +const MAX_HEIGHT = 300; export default class MarkdownTable extends React.PureComponent { static propTypes = { children: PropTypes.node.isRequired, navigator: PropTypes.object.isRequired, + numColumns: PropTypes.number.isRequired, theme: PropTypes.object.isRequired, }; @@ -31,10 +33,16 @@ export default class MarkdownTable extends React.PureComponent { super(props); this.state = { - contentCutOff: true, + containerWidth: 0, + contentHeight: 0, + contentWidth: 0, }; } + getTableWidth = () => { + return this.props.numColumns * CELL_WIDTH; + }; + handlePress = preventDoubleTap(() => { const {navigator, theme} = this.props; @@ -48,6 +56,7 @@ export default class MarkdownTable extends React.PureComponent { backButtonTitle: '', passProps: { renderRows: this.renderRows, + tableWidth: this.getTableWidth(), }, navigatorStyle: { navBarTextColor: theme.sidebarHeaderTextColor, @@ -58,18 +67,25 @@ export default class MarkdownTable extends React.PureComponent { }); }); - handleContentHeightChange = (contentWidth, contentHeight) => { + handleContainerLayout = (e) => { this.setState({ - contentCutOff: contentHeight >= MAX_HEIGHT, + containerWidth: e.nativeEvent.layout.width, }); - } + }; - renderRows = (drawBottomBorder = true) => { + handleContentSizeChange = (contentWidth, contentHeight) => { + this.setState({ + contentHeight, + contentWidth, + }); + }; + + renderRows = (drawExtraBorders = true) => { const style = getStyleSheet(this.props.theme); const tableStyle = [style.table]; - if (drawBottomBorder) { - tableStyle.push(style.tableBottomBorder); + if (drawExtraBorders) { + tableStyle.push(style.tableExtraBorders); } // Add an extra prop to the last row of the table so that it knows not to render a bottom border @@ -89,15 +105,30 @@ export default class MarkdownTable extends React.PureComponent { render() { const style = getStyleSheet(this.props.theme); - let moreIndicator = null; - if (this.state.contentCutOff) { - moreIndicator = ( + let moreRight = null; + if (this.state.containerWidth && this.state.contentWidth > this.state.containerWidth) { + moreRight = ( + ); + } + + let moreBelow = null; + if (this.state.contentHeight > MAX_HEIGHT) { + moreBelow = ( + ); } @@ -105,14 +136,17 @@ export default class MarkdownTable extends React.PureComponent { return ( {this.renderRows(false)} - {moreIndicator} + {moreRight} + {moreBelow} ); } @@ -123,24 +157,31 @@ const getStyleSheet = makeStyleSheetFromTheme((theme) => { container: { borderBottomWidth: 1, borderColor: changeOpacity(theme.centerChannelColor, 0.2), + borderRightWidth: 1, maxHeight: MAX_HEIGHT, }, table: { borderColor: changeOpacity(theme.centerChannelColor, 0.2), - borderLeftWidth: 1, // The right border is drawn by the MarkdownTableCell component + borderLeftWidth: 1, borderTopWidth: 1, - flex: 1, }, - tableBottomBorder: { + tableExtraBorders: { borderBottomWidth: 1, - borderColor: changeOpacity(theme.centerChannelColor, 0.2), + borderRightWidth: 1, }, - moreIndicator: { + moreBelow: { bottom: 0, height: 20, position: 'absolute', right: 0, width: '100%', }, + moreRight: { + height: '100%', + position: 'absolute', + right: 0, + top: 0, + width: 20, + }, }; }); diff --git a/app/components/markdown/markdown_table_cell/markdown_table_cell.js b/app/components/markdown/markdown_table_cell/markdown_table_cell.js index 878eeac0b..52e6496b5 100644 --- a/app/components/markdown/markdown_table_cell/markdown_table_cell.js +++ b/app/components/markdown/markdown_table_cell/markdown_table_cell.js @@ -7,10 +7,13 @@ import {Text, View} from 'react-native'; import {changeOpacity, makeStyleSheetFromTheme} from 'app/utils/theme'; +export const CELL_WIDTH = 100; + export default class MarkdownTableCell extends React.PureComponent { static propTypes = { align: PropTypes.oneOf(['', 'left', 'center', 'right']), children: PropTypes.node, + isLastCell: PropTypes.bool, theme: PropTypes.object.isRequired, }; @@ -18,6 +21,9 @@ export default class MarkdownTableCell extends React.PureComponent { const style = getStyleSheet(this.props.theme); const cellStyle = [style.cell]; + if (!this.props.isLastCell) { + cellStyle.push(style.cellRightBorder); + } let textStyle = null; if (this.props.align === 'center') { @@ -40,12 +46,14 @@ const getStyleSheet = makeStyleSheetFromTheme((theme) => { return { cell: { borderColor: changeOpacity(theme.centerChannelColor, 0.2), - borderRightWidth: 1, - flex: 1, + width: CELL_WIDTH, justifyContent: 'flex-start', paddingHorizontal: 13, paddingVertical: 6, }, + cellRightBorder: { + borderRightWidth: 1, + }, alignCenter: { textAlign: 'center', }, diff --git a/app/components/markdown/markdown_table_row/markdown_table_row.js b/app/components/markdown/markdown_table_row/markdown_table_row.js index a23f01a9a..9bb5ae776 100644 --- a/app/components/markdown/markdown_table_row/markdown_table_row.js +++ b/app/components/markdown/markdown_table_row/markdown_table_row.js @@ -22,16 +22,21 @@ export default class MarkdownTableRow extends React.PureComponent { rowStyle.push(style.rowBottomBorder); } - return {this.props.children}; + // Add an extra prop to the last cell so that it knows not to render a right border since the container + // will handle that + const children = React.Children.toArray(this.props.children); + children[children.length - 1] = React.cloneElement(children[children.length - 1], { + isLastCell: true, + }); + + return {children}; } } const getStyleSheet = makeStyleSheetFromTheme((theme) => { return { row: { - flex: 1, flexDirection: 'row', - justifyContent: 'flex-start', }, rowBottomBorder: { borderColor: changeOpacity(theme.centerChannelColor, 0.2), diff --git a/app/screens/table/table.js b/app/screens/table/table.js index e1b004e35..1dd6f15cf 100644 --- a/app/screens/table/table.js +++ b/app/screens/table/table.js @@ -3,30 +3,39 @@ import PropTypes from 'prop-types'; import React from 'react'; -import {ScrollView, StyleSheet} from 'react-native'; +import { + Platform, + ScrollView, +} from 'react-native'; export default class Table extends React.PureComponent { static propTypes = { renderRows: PropTypes.func.isRequired, + tableWidth: PropTypes.number.isRequired, }; render() { - return ( - - {this.props.renderRows()} - - ); + const content = this.props.renderRows(); + + let container; + if (Platform.OS === 'android') { + // On Android, ScrollViews can only handle one direction at once, so use two ScrollViews that go in + // different directions. This prevents diagonal scrolling, so only do it on Android when totally necessary. + container = ( + + + {content} + + + ); + } else { + container = ( + + {content} + + ); + } + + return container; } } - -const style = StyleSheet.create({ - scrollContainer: { - flex: 1, - }, - container: { - flexDirection: 'row', - }, -}); diff --git a/package-lock.json b/package-lock.json index a4feaa757..fdd763c8d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2840,8 +2840,8 @@ } }, "commonmark-react-renderer": { - "version": "github:mattermost/commonmark-react-renderer#ea502501b1f2c31c6b3ff6d40eead2c808f40d42", - "from": "github:mattermost/commonmark-react-renderer#ea502501b1f2c31c6b3ff6d40eead2c808f40d42", + "version": "github:mattermost/commonmark-react-renderer#3a2ac19cab725ad28b170fdc1d397dddedcf87eb", + "from": "github:mattermost/commonmark-react-renderer#3a2ac19cab725ad28b170fdc1d397dddedcf87eb", "requires": { "in-publish": "^2.0.0", "lodash.assign": "^4.2.0", diff --git a/package.json b/package.json index a1b47c35e..a98016228 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "@babel/runtime": "7.1.5", "analytics-react-native": "1.2.0", "commonmark": "github:mattermost/commonmark.js#d5a7a7bfb373778d3bfd4575962c10fb3f3909c6", - "commonmark-react-renderer": "github:mattermost/commonmark-react-renderer#ea502501b1f2c31c6b3ff6d40eead2c808f40d42", + "commonmark-react-renderer": "github:mattermost/commonmark-react-renderer#3a2ac19cab725ad28b170fdc1d397dddedcf87eb", "deep-equal": "1.0.1", "fuse.js": "3.3.0", "intl": "1.2.5",