Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate data that is expanded #7777

Merged
merged 3 commits into from Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 34 additions & 12 deletions superset/assets/src/components/FilterableTable/FilterableTable.jsx
Expand Up @@ -115,6 +115,7 @@ export default class FilterableTable extends PureComponent {
super(props);
this.list = List(this.formatTableData(props.data));
this.addJsonModal = this.addJsonModal.bind(this);
this.getCellContent = this.getCellContent.bind(this);
this.renderGridCell = this.renderGridCell.bind(this);
this.renderGridCellHeader = this.renderGridCellHeader.bind(this);
this.renderGrid = this.renderGrid.bind(this);
Expand Down Expand Up @@ -152,21 +153,32 @@ export default class FilterableTable extends PureComponent {
const widthsByColumnKey = {};
this.props.orderedColumnKeys.forEach((key) => {
const colWidths = this.list
.map(d => getTextWidth(d[key]) + PADDING) // get width for each value for a key
.push(getTextWidth(key) + PADDING); // add width of column key to end of list
// get width for each value for a key
.map(d => getTextWidth(
this.getCellContent({ cellData: d[key], columnKey: key })) + PADDING,
)
// add width of column key to end of list
.push(getTextWidth(key) + PADDING);
// set max width as value for key
widthsByColumnKey[key] = Math.max(...colWidths);
});
return widthsByColumnKey;
}

fitTableToWidthIfNeeded() {
const containerWidth = this.container.clientWidth;
if (this.totalTableWidth < containerWidth) {
// fit table width if content doesn't fill the width of the container
this.totalTableWidth = containerWidth;
getCellContent({ cellData, columnKey }) {
const complexColumn = this.props.expandedColumns.some(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should do this outside of getCellContent and pass it in since you'd be doing this loop through all the columns for each row in the table

name => name.startsWith(columnKey + '.'),
);
const content = String(cellData);
let type;
if (content.substring(0, 1) === '[') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is over optimization at this point, but since this function could be called tens of thousands of times, it might be better to set content.substring(0,1) to a const instead of calling it twice.

type = '[…]';
} else if (content.substring(0, 1) === '{') {
type = '{…}';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would we hit this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it was supposed to be content.substring(0, 1) === '{'. Will fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the content doesn't distinguish between rows and arrays. A row content will also be "[ "a", "b", ... ]".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, but I think it's fine to show the rows as [...], since we represent the data as Javascript arrays.

Copy link
Contributor

@khtruong khtruong Jun 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. We can shorten this logic then.

const type = content.substring(0, 1) === '[' ? '[...]' : ''

} else {
type = '';
}
this.setState({ fitted: true });
return complexColumn ? type : content;
}

formatTableData(data) {
Expand Down Expand Up @@ -213,6 +225,15 @@ export default class FilterableTable extends PureComponent {
this.setState({ sortBy, sortDirection });
}

fitTableToWidthIfNeeded() {
const containerWidth = this.container.clientWidth;
if (this.totalTableWidth < containerWidth) {
// fit table width if content doesn't fill the width of the container
this.totalTableWidth = containerWidth;
}
this.setState({ fitted: true });
}

addJsonModal(node, jsonObject, jsonString) {
return (
<ModalTrigger
Expand Down Expand Up @@ -260,13 +281,14 @@ export default class FilterableTable extends PureComponent {
renderGridCell({ columnIndex, key, rowIndex, style }) {
const columnKey = this.props.orderedColumnKeys[columnIndex];
const cellData = this.list.get(rowIndex)[columnKey];
const content = this.getCellContent({ cellData, columnKey });
const cellNode = (
<div
key={key}
style={{ ...style, top: style.top - GRID_POSITION_ADJUSTMENT }}
className={`grid-cell ${this.rowClassName({ index: rowIndex })}`}
>
{cellData}
{content}
</div>
);

Expand Down Expand Up @@ -332,8 +354,8 @@ export default class FilterableTable extends PureComponent {
);
}

renderTableCell({ cellData }) {
const cellNode = String(cellData);
renderTableCell({ cellData, columnKey }) {
const cellNode = this.getCellContent({ cellData, columnKey });
const jsonObject = safeJsonObjectParse(cellData);
if (jsonObject) {
return this.addJsonModal(cellNode, jsonObject, cellData);
Expand Down Expand Up @@ -396,7 +418,7 @@ export default class FilterableTable extends PureComponent {
>
{orderedColumnKeys.map(columnKey => (
<Column
cellRenderer={this.renderTableCell}
cellRenderer={({ cellData }) => this.renderTableCell({ cellData, columnKey })}
dataKey={columnKey}
disableSort={false}
headerRenderer={this.renderTableHeader}
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/presto.py
Expand Up @@ -552,7 +552,7 @@ def _process_array_data(cls,
{'ColumnA': [1, 2], 'ColumnB': [3]},
{'ColumnA': [11, 22], 'ColumnB': [33]}
]
all_array_data (intially) = {
all_array_data (initially) = {
0: [{'ColumnA': [1, 2], 'ColumnB': [3}],
1: [{'ColumnA': [11, 22], 'ColumnB': [33]}]
}
Expand Down