Skip to content

Commit

Permalink
fix(legacy-plugin-chart-pivot-table): fix displaying image inside rows (
Browse files Browse the repository at this point in the history 
#954)

* fix

* lint

* lint

* complete fix

* remove log

* Add storybook entry for html

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>
  • Loading branch information
@mayurnewase @ktmud @zhaoyongjie
2 people authored and zhaoyongjie committed Nov 26, 2021
1 parent f11f2eb commit 9357d2b
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 50 deletions.
77 changes: 75 additions & 2 deletions ...es/superset-ui-demo/storybook/stories/plugins/legacy-plugin-chart-pivot-table/Stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,81 @@ export const basic = () => (
['sum__num', 'other'],
['sum__num', 'All'],
],
html:
'<table border="1" class="dataframe dataframe table table-striped table-bordered table-condensed table-hover">\n <thead>\n <tr>\n <th></th>\n <th colspan="2" halign="left">sum__num</th>\n </tr>\n <tr>\n <th>state</th>\n <th>other</th>\n <th>All</th>\n </tr>\n <tr>\n <th>name</th>\n <th></th>\n <th></th>\n </tr>\n </thead>\n <tbody>\n <tr>\n <th>Christopher</th>\n <td>803607</td>\n <td>803607</td>\n </tr>\n <tr>\n <th>David</th>\n <td>673992</td>\n <td>673992</td>\n </tr>\n <tr>\n <th>James</th>\n <td>749686</td>\n <td>749686</td>\n </tr>\n <tr>\n <th>Jennifer</th>\n <td>587540</td>\n <td>587540</td>\n </tr>\n <tr>\n <th>John</th>\n <td>638450</td>\n <td>638450</td>\n </tr>\n <tr>\n <th>Joshua</th>\n <td>548044</td>\n <td>548044</td>\n </tr>\n <tr>\n <th>Matthew</th>\n <td>608212</td>\n <td>608212</td>\n </tr>\n <tr>\n <th>Michael</th>\n <td>1047996</td>\n <td>1047996</td>\n </tr>\n <tr>\n <th>Robert</th>\n <td>575592</td>\n <td>575592</td>\n </tr>\n <tr>\n <th>William</th>\n <td>574464</td>\n <td>574464</td>\n </tr>\n <tr>\n <th>All</th>\n <td>6807583</td>\n <td>6807583</td>\n </tr>\n </tbody>\n</table>',
html: `<table border="1" class="dataframe dataframe table table-striped table-bordered table-condensed table-hover">
<thead>
<tr>
<th></th>
<th colspan="2" halign="left">sum__num</th>
</tr>
<tr>
<th>state</th>
<th>other</th>
<th>All</th>
</tr>
<tr>
<th>name</th>
<th></th>
<th></th>
</tr>
</thead>
<tbody>
<tr>
<th><a href="https://superset.apache.org">Apache Superset</a></th>
<td>803607</td>
<td>803607</td>
</tr>
<tr>
<th>David <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mMUXn76JAAEqgJQrlGqUwAAAABJRU5ErkJggg==" width="20" height="20" alt="pixel" /></th>
<td>673992</td>
<td>673992</td>
</tr>
<tr>
<th><a href="https://superset.apache.org" target="_blank"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mMUXn76JAAEqgJQrlGqUwAAAABJRU5ErkJggg==" width="20" height="20" alt="pixel" />Apache Superset</a></th>
<td>749686</td>
<td>749686</td>
</tr>
<tr>
<th>Jennifer</th>
<td>587540</td>
<td>587540</td>
</tr>
<tr>
<th>John</th>
<td>638450</td>
<td>638450</td>
</tr>
<tr>
<th>Joshua</th>
<td>548044</td>
<td>548044</td>
</tr>
<tr>
<th>Matthew</th>
<td>608212</td>
<td>608212</td>
</tr>
<tr>
<th>Michael</th>
<td>1047996</td>
<td>1047996</td>
</tr>
<tr>
<th>Robert</th>
<td>575592</td>
<td>575592</td>
</tr>
<tr>
<th>William</th>
<td>574464</td>
<td>574464</td>
</tr>
<tr>
<th>All</th>
<td>6807583</td>
<td>6807583</td>
</tr>
</tbody>
</table>`,
},
},
]}
Expand Down
52 changes: 22 additions & 30 deletions ...mporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ const propTypes = {
verboseMap: PropTypes.objectOf(PropTypes.string),
};

const hasOnlyTextChild = node =>
node.childNodes.length === 1 && node.childNodes[0].nodeType === Node.TEXT_NODE;

function PivotTable(element, props) {
const {
columnFormats,
Expand Down Expand Up @@ -79,42 +82,31 @@ function PivotTable(element, props) {
const cols = Array.isArray(columns[0]) ? columns.map(col => col[0]) : columns;
const dateRegex = /^__timestamp:(-?\d*\.?\d*)$/;

$container.find('thead tr th').each(function () {
const cellValue = formatDateCellValue(
$(this)[0].textContent,
verboseMap,
dateRegex,
dateFormatter,
);
$(this)[0].textContent = cellValue;
});

$container.find('tbody tr th').each(function () {
const cellValue = formatDateCellValue(
$(this)[0].textContent,
verboseMap,
dateRegex,
dateFormatter,
);
$(this)[0].textContent = cellValue;
$container.find('th').each(function formatTh() {
if (hasOnlyTextChild(this)) {
const cellValue = formatDateCellValue($(this).text(), verboseMap, dateRegex, dateFormatter);
$(this).text(cellValue);
}
});

$container.find('tbody tr').each(function eachRow() {
$(this)
.find('td')
.each(function eachTd(index) {
const tdText = $(this)[0].textContent;
const { textContent, attr } = formatCellValue(
index,
cols,
tdText,
columnFormats,
numberFormat,
dateRegex,
dateFormatter,
);
$(this)[0].textContent = textContent;
$(this).attr = attr;
if (hasOnlyTextChild(this)) {
const tdText = $(this).text();
const { textContent, sortAttributeValue } = formatCellValue(
index,
cols,
tdText,
columnFormats,
numberFormat,
dateRegex,
dateFormatter,
);
$(this).text(textContent);
$(this).attr('data-sort', sortAttributeValue);
}
});
});

Expand Down
4 changes: 1 addition & 3 deletions ..._superset_ui/superset-ui/plugins/legacy-plugin-chart-pivot-table/src/utils/formatCells.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ function formatCellValue(
sortAttributeValue = Number.NEGATIVE_INFINITY;
}
}
// @ts-ignore
const attr = ('data-sort', sortAttributeValue);

return { textContent, attr };
return { textContent, sortAttributeValue };
}

function formatDateCellValue(text: string, verboseMap: any, dateRegex: RegExp, dateFormatter: any) {
Expand Down
12 changes: 6 additions & 6 deletions ...y_superset_ui/superset-ui/plugins/legacy-plugin-chart-pivot-table/test/PivotTable.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('pivot table plugin format cells', () => {
const dateFormatter = getTimeFormatterForGranularity('P1D');

it('render number', () => {
const { textContent, attr } = formatCellValue(
const { textContent, sortAttributeValue } = formatCellValue(
i,
cols,
tdText,
Expand All @@ -39,7 +39,7 @@ describe('pivot table plugin format cells', () => {
dateFormatter,
);
expect(textContent).toEqual('2.22M');
expect(attr).toEqual(('data-sort', 2222222));
expect(sortAttributeValue).toEqual(2222222);
});

it('render date', () => {
Expand All @@ -60,7 +60,7 @@ describe('pivot table plugin format cells', () => {
it('render string', () => {
tdText = 'some-text';

const { textContent, attr } = formatCellValue(
const { textContent, sortAttributeValue } = formatCellValue(
i,
cols,
tdText,
Expand All @@ -70,13 +70,13 @@ describe('pivot table plugin format cells', () => {
dateFormatter,
);
expect(textContent).toEqual(tdText);
expect(attr).toEqual(('data-sort', tdText));
expect(sortAttributeValue).toEqual(tdText);
});

it('render null', () => {
tdText = 'null';

const { textContent, attr } = formatCellValue(
const { textContent, sortAttributeValue } = formatCellValue(
i,
cols,
tdText,
Expand All @@ -86,6 +86,6 @@ describe('pivot table plugin format cells', () => {
dateFormatter,
);
expect(textContent).toEqual('');
expect(attr).toEqual(('data-sort', Number.NEGATIVE_INFINITY));
expect(sortAttributeValue).toEqual(Number.NEGATIVE_INFINITY);
});
});
10 changes: 1 addition & 9 deletions superset-frontend/temporary_superset_ui/superset-ui/yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4740,14 +4740,6 @@
jest-diff "^26.0.0"
pretty-format "^26.0.0"

"@types/jest@^26.0.0":
version "26.0.15"
resolved "https://registry.yarnpkg.com/@types/jest/-/jest-26.0.15.tgz#12e02c0372ad0548e07b9f4e19132b834cb1effe"
integrity sha512-s2VMReFXRg9XXxV+CW9e5Nz8fH2K1aEhwgjUqPPbQd7g95T0laAcvLv032EhFHIa5GHsZ8W7iJEQVaJq6k3Gog==
dependencies:
jest-diff "^26.0.0"
pretty-format "^26.0.0"

"@types/jest@^26.0.4":
version "26.0.13"
resolved "https://registry.yarnpkg.com/@types/jest/-/jest-26.0.13.tgz#5a7b9d5312f5dd521a38329c38ee9d3802a0b85e"
Expand Down Expand Up @@ -14667,7 +14659,7 @@ jest@^25.5.4:
import-local "^3.0.2"
jest-cli "^25.5.4"

jest@^26.0.1, jest@^26.6.3:
jest@^26.6.3:
version "26.6.3"
resolved "https://registry.yarnpkg.com/jest/-/jest-26.6.3.tgz#40e8fdbe48f00dfa1f0ce8121ca74b88ac9148ef"
integrity sha512-lGS5PXGAzR4RF7V5+XObhqz2KZIDUA1yD0DG6pBVmy10eh0ZIXQImRuzocsI/N2XZ1GrLFwTS27In2i2jlpq1Q==
Expand Down

0 comments on commit 9357d2b

Please sign in to comment.