From f7bed8dcdb9f56778cfea4b838cc9b7e7f7c1041 Mon Sep 17 00:00:00 2001 From: Andre Vasconcelos Date: Sun, 1 Dec 2019 02:27:02 +0900 Subject: [PATCH] MM-15635 - Making markdown tables more responsive (#3249) * Making markdown tables more responsive * Changing implementation of isTablet and isLandscape * Fixing variable names & added more table styles * Adding header backgrounds & iOS fixes * Changing cell padding to 8 * Fixing positioning of expand table buttons * Fixed android issues and flex render conditions - Centered icon in expand button instead of using fixed paddings (icon was slightly off center) - Fixed expand button rendering on android - Separated full table view into ios / android to allow for separate styling * Merging table back to single file * Resolving conflicts & improving full table view - Made the full table view scrollable through any part of the screen, as opposed to only the table area (only works on iOS) - Resolved conflicts with PR that limits tables to 5 columns visible on channel view - Made changes to keep behavior of tables consistent on table view - Added logic to render "moreRight" element if table was cut in channel view - Updated snapshot --- .../__snapshots__/markdown_table.test.js.snap | 80 ++++++---- .../markdown/markdown_table/markdown_table.js | 137 ++++++++++++------ .../markdown_table_cell.js | 8 +- .../markdown_table_row/markdown_table_row.js | 9 ++ app/screens/table/table.js | 44 +++++- 5 files changed, 200 insertions(+), 78 deletions(-) diff --git a/app/components/markdown/markdown_table/__snapshots__/markdown_table.test.js.snap b/app/components/markdown/markdown_table/__snapshots__/markdown_table.test.js.snap index cd6d30b2e..cb7a2b7c8 100644 --- a/app/components/markdown/markdown_table/__snapshots__/markdown_table.test.js.snap +++ b/app/components/markdown/markdown_table/__snapshots__/markdown_table.test.js.snap @@ -11,27 +11,14 @@ exports[`MarkdownTable should match snapshot 1`] = ` type="opacity" > @@ -144,16 +133,56 @@ exports[`MarkdownTable should match snapshot 1`] = ` + { - const maxPreviewColumns = Math.floor(window.width / CELL_WIDTH); + const maxPreviewColumns = Math.floor(window.width / CELL_MIN_WIDTH); this.setState({maxPreviewColumns}); } - getTableWidth = () => { - return this.props.numColumns * CELL_WIDTH; + getTableWidth = (isFullView = false) => { + let columns = this.props.numColumns; + + if (columns > MAX_PREVIEW_COLUMNS) { + columns = MAX_PREVIEW_COLUMNS; + } + + return isFullView || columns === 1 ? columns * CELL_MAX_WIDTH : columns * CELL_MIN_WIDTH; }; handlePress = preventDoubleTap(() => { @@ -71,7 +78,8 @@ export default class MarkdownTable extends React.PureComponent { }); const passProps = { renderRows: this.renderRows, - tableWidth: this.getTableWidth(), + tableWidth: this.getTableWidth(true), + renderAsFlex: this.shouldRenderAsFlex(true), }; goToScreen(screen, title, passProps); @@ -86,18 +94,12 @@ export default class MarkdownTable extends React.PureComponent { handleContentSizeChange = (contentWidth, contentHeight) => { this.setState({ contentHeight, - contentWidth, }); }; - renderPreviewRows = (drawExtraBorders = true) => { + renderPreviewRows = (isFullView = false) => { const {maxPreviewColumns} = this.state; - const style = getStyleSheet(this.props.theme); - - const tableStyle = [style.table]; - if (drawExtraBorders) { - tableStyle.push(style.tableExtraBorders); - } + const tableStyle = this.getTableStyle(isFullView); // Add an extra prop to the last row of the table so that it knows not to render a bottom border // since the container should be rendering that @@ -122,14 +124,50 @@ export default class MarkdownTable extends React.PureComponent { ); } - renderRows = (drawExtraBorders = true) => { - const style = getStyleSheet(this.props.theme); + shouldRenderAsFlex = (isFullView = false) => { + const {numColumns} = this.props; + const {height, width} = Dimensions.get('window'); + const isLandscape = width > height; - const tableStyle = [style.table]; - if (drawExtraBorders) { - tableStyle.push(style.tableExtraBorders); + // render as flex in the channel screen, only for mobile phones on the portrait mode, + // and if tables have 2 ~ 4 columns + if (!isFullView && numColumns > 1 && numColumns < 4 && !DeviceTypes.IS_TABLET) { + return true; } + // render a 4 column table as flex when in landscape mode only + // otherwise it should expand beyond the device's full width + if (!isFullView && isLandscape && numColumns === 4) { + return true; + } + + // render as flex in full table screen, only for mobile phones on portrait mode, + // and if tables have 3 or 4 columns + if (isFullView && numColumns >= 3 && numColumns <= 4 && !DeviceTypes.IS_TABLET) { + return true; + } + + return false; + } + + getTableStyle = (isFullView) => { + const {theme} = this.props; + const style = getStyleSheet(theme); + const tableStyle = [style.table]; + + const renderAsFlex = this.shouldRenderAsFlex(isFullView); + if (renderAsFlex) { + tableStyle.push(style.displayFlex); + return tableStyle; + } + + tableStyle.push({width: this.getTableWidth(isFullView)}); + return tableStyle; + } + + renderRows = (isFullView = false) => { + const tableStyle = this.getTableStyle(isFullView); + // Add an extra prop to the last row of the table so that it knows not to render a bottom border // since the container should be rendering that const rows = React.Children.toArray(this.props.children); @@ -137,6 +175,11 @@ export default class MarkdownTable extends React.PureComponent { isLastRow: true, }); + // Add an extra prop to the first row of the table so that it can have a different background color + rows[0] = React.cloneElement(rows[0], { + isFirstRow: true, + }); + return ( {rows} @@ -146,9 +189,20 @@ export default class MarkdownTable extends React.PureComponent { render() { const style = getStyleSheet(this.props.theme); - let moreRight = null; - if (this.state.containerWidth && this.state.contentWidth > this.state.containerWidth) { + const tableWidth = this.getTableWidth(); + const renderAsFlex = this.shouldRenderAsFlex(); + + let leftOffset; + if (renderAsFlex || tableWidth > this.state.containerWidth) { + leftOffset = this.state.containerWidth - 20; + } else { + leftOffset = tableWidth - 20; + } + + // Renders when table width exceeds the container, or if the columns exceed maximum allowed for previews + if ((this.state.containerWidth && tableWidth > this.state.containerWidth && !renderAsFlex) || + (this.props.numColumns > MAX_PREVIEW_COLUMNS)) { moreRight = ( ); } @@ -170,15 +224,16 @@ export default class MarkdownTable extends React.PureComponent { changeOpacity(this.props.theme.centerChannelColor, 0.0), changeOpacity(this.props.theme.centerChannelColor, 0.1), ]} - style={[style.moreBelow, {width: this.getTableWidth()}]} + style={[style.moreBelow, renderAsFlex ? style.fullWidth : {width: tableWidth}]} /> ); } const expandButton = ( @@ -198,14 +253,13 @@ export default class MarkdownTable extends React.PureComponent { type={'opacity'} > - {this.renderPreviewRows(false)} + {this.renderPreviewRows()} {moreRight} {moreBelow} @@ -218,9 +272,6 @@ export default class MarkdownTable extends React.PureComponent { const getStyleSheet = makeStyleSheetFromTheme((theme) => { return { container: { - borderBottomWidth: 1, - borderColor: changeOpacity(theme.centerChannelColor, 0.2), - borderRightWidth: 1, maxHeight: MAX_HEIGHT, }, expandButton: { @@ -243,12 +294,13 @@ const getStyleSheet = makeStyleSheetFromTheme((theme) => { marginTop: -32, marginRight: -6, borderWidth: 1, + display: 'flex', + justifyContent: 'center', + alignItems: 'center', borderRadius: 50, borderColor: changeOpacity(theme.centerChannelColor, 0.2), width: 34, height: 34, - alignItems: 'center', - justifyContent: 'center', }, icon: { fontSize: 14, @@ -259,32 +311,35 @@ const getStyleSheet = makeStyleSheetFromTheme((theme) => { }, }), }, + displayFlex: { + flex: 1, + }, + fullWidth: { + width: '100%', + }, table: { width: '100%', borderColor: changeOpacity(theme.centerChannelColor, 0.2), - borderLeftWidth: 1, - borderTopWidth: 1, + borderWidth: 1, }, tablePadding: { paddingRight: 10, }, - tableExtraBorders: { - borderBottomWidth: 1, - borderRightWidth: 1, - }, moreBelow: { bottom: 30, height: 20, position: 'absolute', left: 0, - width: '100%', + borderColor: changeOpacity(theme.centerChannelColor, 0.2), + borderBottomWidth: 1, }, moreRight: { maxHeight: MAX_HEIGHT, position: 'absolute', - right: 10, top: 0, width: 20, + borderColor: changeOpacity(theme.centerChannelColor, 0.2), + borderRightWidth: 1, }, }; }); 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 52e6496b5..b5ac917de 100644 --- a/app/components/markdown/markdown_table_cell/markdown_table_cell.js +++ b/app/components/markdown/markdown_table_cell/markdown_table_cell.js @@ -7,7 +7,8 @@ import {Text, View} from 'react-native'; import {changeOpacity, makeStyleSheetFromTheme} from 'app/utils/theme'; -export const CELL_WIDTH = 100; +export const CELL_MIN_WIDTH = 96; +export const CELL_MAX_WIDTH = 192; export default class MarkdownTableCell extends React.PureComponent { static propTypes = { @@ -45,11 +46,10 @@ export default class MarkdownTableCell extends React.PureComponent { const getStyleSheet = makeStyleSheetFromTheme((theme) => { return { cell: { + flex: 1, borderColor: changeOpacity(theme.centerChannelColor, 0.2), - width: CELL_WIDTH, justifyContent: 'flex-start', - paddingHorizontal: 13, - paddingVertical: 6, + padding: 8, }, cellRightBorder: { borderRightWidth: 1, 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 9bb5ae776..ca4a38afa 100644 --- a/app/components/markdown/markdown_table_row/markdown_table_row.js +++ b/app/components/markdown/markdown_table_row/markdown_table_row.js @@ -11,6 +11,7 @@ export default class MarkdownTableRow extends React.PureComponent { static propTypes = { children: PropTypes.node, isLastRow: PropTypes.bool, + isFirstRow: PropTypes.bool, theme: PropTypes.object.isRequired, }; @@ -22,6 +23,10 @@ export default class MarkdownTableRow extends React.PureComponent { rowStyle.push(style.rowBottomBorder); } + if (this.props.isFirstRow) { + rowStyle.push(style.rowTopBackground); + } + // 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); @@ -36,8 +41,12 @@ export default class MarkdownTableRow extends React.PureComponent { const getStyleSheet = makeStyleSheetFromTheme((theme) => { return { row: { + flex: 1, flexDirection: 'row', }, + rowTopBackground: { + backgroundColor: changeOpacity(theme.centerChannelColor, 0.1), + }, rowBottomBorder: { borderColor: changeOpacity(theme.centerChannelColor, 0.2), borderBottomWidth: 1, diff --git a/app/screens/table/table.js b/app/screens/table/table.js index 1dd6f15cf..9a97d6e1b 100644 --- a/app/screens/table/table.js +++ b/app/screens/table/table.js @@ -6,36 +6,64 @@ import React from 'react'; import { Platform, ScrollView, + SafeAreaView, } from 'react-native'; +import {makeStyleSheetFromTheme} from 'app/utils/theme'; export default class Table extends React.PureComponent { static propTypes = { renderRows: PropTypes.func.isRequired, tableWidth: PropTypes.number.isRequired, + renderAsFlex: PropTypes.bool.isRequired, }; render() { - const content = this.props.renderRows(); + const style = getStyleSheet(); + const content = this.props.renderRows(true); + const viewStyle = this.props.renderAsFlex ? style.displayFlex : {width: this.props.tableWidth}; 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} - + + + {content} + + ); } - return container; } } + +const getStyleSheet = makeStyleSheetFromTheme(() => { + return { + fullHeight: { + height: '100%', + }, + displayFlex: { + ...Platform.select({ + android: { + flex: 1, + }, + ios: { + flex: 0, + }, + }), + }, + }; +});