This is an automated email from the ASF dual-hosted git repository. villebro pushed a commit to branch 0.38 in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit cfd86918ff345ce1b9579daf504f8ca3b2a1acbe Author: ʈᵃᵢ <[email protected]> AuthorDate: Fri Aug 21 10:32:37 2020 -0700 feat(listview): skeleton loading states for table and card collections (#10606) --- superset-frontend/.eslintrc.js | 21 ++++++ superset-frontend/package-lock.json | 50 +++++++++----- superset-frontend/package.json | 3 +- superset-frontend/spec/.eslintrc | 7 +- .../views/CRUD/chart/ChartList_spec.jsx | 4 ++ .../views/CRUD/dashboard/DashboardList_spec.jsx | 4 ++ .../src/common/components/{index.js => index.ts} | 13 ++++ .../src/components/Label/Label.test.tsx | 2 - .../src/components/ListView/CardCollection.tsx | 53 +++++++++----- .../src/components/ListView/ListView.tsx | 15 ++-- .../src/components/ListView/TableCollection.tsx | 65 ++++++++++++------ .../components/ListViewCard/ImageLoader.test.jsx | 73 ++++++++++++++++++++ .../src/components/ListViewCard/ImageLoader.tsx | 64 +++++++++++++++++ .../ListViewCard/ListViewCard.stories.tsx | 20 +++++- .../components/ListViewCard/ListViewCard.test.jsx | 69 +++++++++++++++++++ .../src/components/ListViewCard/index.tsx | 80 ++++++++++++++++------ .../src/views/CRUD/chart/ChartList.tsx | 3 +- .../src/views/CRUD/dashboard/DashboardList.tsx | 5 +- 18 files changed, 456 insertions(+), 95 deletions(-) diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js index 546ac6c..15c183c 100644 --- a/superset-frontend/.eslintrc.js +++ b/superset-frontend/.eslintrc.js @@ -131,6 +131,27 @@ module.exports = { ], }, }, + { + files: [ + 'src/**/*.test.ts', + 'src/**/*.test.tsx', + 'src/**/*.test.js', + 'src/**/*.test.jsx', + ], + plugins: ['jest', 'no-only-tests'], + env: { + 'jest/globals': true, + }, + extends: ['plugin:jest/recommended'], + rules: { + 'import/no-extraneous-dependencies': [ + 'error', + { devDependencies: true }, + ], + 'jest/consistent-test-it': 'error', + 'no-only-tests/no-only-tests': 'error', + }, + }, ], rules: { camelcase: [ diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 43bb6e3..e3512e9 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -20142,9 +20142,9 @@ }, "dependencies": { "core-js": { - "version": "2.6.0", - "resolved": "https://registry.npmjs.org/core-js/-/core-js-2.6.0.tgz";, - "integrity": "sha512-kLRC6ncVpuEW/1kwrOXYX6KQASCVtrh1gQr/UiaVgFlf9WE5Vp+lNe5+h3LuMr5PAucWnnEXwH0nQHRH/gpGtw==", + "version": "2.6.11", + "resolved": "https://registry.npmjs.org/core-js/-/core-js-2.6.11.tgz";, + "integrity": "sha512-5wjnpaT/3dV+XB4borEsnAYQchn00XSgTAWKDkEqv+K8KevjbzmofK6hfJ9TZIlpj2N0xQpazy7PiRQiWHqzWg==", "dev": true }, "regenerator-runtime": { @@ -25861,15 +25861,25 @@ } }, "fetch-mock": { - "version": "7.2.5", - "resolved": "https://registry.npmjs.org/fetch-mock/-/fetch-mock-7.2.5.tgz";, - "integrity": "sha512-ZdlNxw2xFE2VuGikqWYBcshbfMtWM0k7zWevYgjrFuTiJ1+S7+xjRMxDG1cy45xkpEcqzZAAeqL+uDL5qLZV7g==", + "version": "7.7.3", + "resolved": "https://registry.npmjs.org/fetch-mock/-/fetch-mock-7.7.3.tgz";, + "integrity": "sha512-I4OkK90JFQnjH8/n3HDtWxH/I6D1wrxoAM2ri+nb444jpuH3RTcgvXx2el+G20KO873W727/66T7QhOvFxNHPg==", "dev": true, "requires": { "babel-polyfill": "^6.26.0", + "core-js": "^2.6.9", "glob-to-regexp": "^0.4.0", + "lodash.isequal": "^4.5.0", "path-to-regexp": "^2.2.1", "whatwg-url": "^6.5.0" + }, + "dependencies": { + "core-js": { + "version": "2.6.11", + "resolved": "https://registry.npmjs.org/core-js/-/core-js-2.6.11.tgz";, + "integrity": "sha512-5wjnpaT/3dV+XB4borEsnAYQchn00XSgTAWKDkEqv+K8KevjbzmofK6hfJ9TZIlpj2N0xQpazy7PiRQiWHqzWg==", + "dev": true + } } }, "fetch-retry": { @@ -26564,9 +26574,9 @@ } }, "glob-to-regexp": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/glob-to-regexp/-/glob-to-regexp-0.4.0.tgz";, - "integrity": "sha512-fyPCII4vn9Gvjq2U/oDAfP433aiE64cyP/CJjRJcpVGjqqNdioUYn9+r0cSzT1XPwmGAHuTT7iv+rQT8u/YHKQ==", + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/glob-to-regexp/-/glob-to-regexp-0.4.1.tgz";, + "integrity": "sha512-lkX1HJXwyMcprw/5YUZc2s7DrpAiHB21/V+E1rHUrVNokkvB6bqMzT0VfV6/86ZNabt1k14YOIaT7nDvOX3Iiw==", "dev": true }, "global": { @@ -28307,6 +28317,17 @@ "requires": { "node-fetch": "^1.0.1", "whatwg-fetch": ">=0.10.0" + }, + "dependencies": { + "node-fetch": { + "version": "1.7.3", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-1.7.3.tgz";, + "integrity": "sha512-NhZ4CsKx7cYm2vSrBAr2PvFOe6sWDf0UYLRqA6svUYg7+/TSfVAu49jYC4BvQ4Sms9SZgdqGBgroqfDhJdTyKQ==", + "requires": { + "encoding": "^0.1.11", + "is-stream": "^1.0.1" + } + } } }, "isstream": { @@ -33828,13 +33849,10 @@ } }, "node-fetch": { - "version": "1.7.3", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-1.7.3.tgz";, - "integrity": "sha512-NhZ4CsKx7cYm2vSrBAr2PvFOe6sWDf0UYLRqA6svUYg7+/TSfVAu49jYC4BvQ4Sms9SZgdqGBgroqfDhJdTyKQ==", - "requires": { - "encoding": "^0.1.11", - "is-stream": "^1.0.1" - } + "version": "2.6.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.0.tgz";, + "integrity": "sha512-8dG4H5ujfvFiqDmVu9fQ5bOHUC15JMjMY/Zumv26oOvvVJjM67KF8koCWIabKQ1GJIa9r2mMZscBq/TbdOcmNA==", + "dev": true }, "node-forge": { "version": "0.9.0", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 28f0cb8..cb00328 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -255,7 +255,7 @@ "eslint-plugin-prettier": "^3.1.3", "eslint-plugin-react": "^7.16.0", "exports-loader": "^0.7.0", - "fetch-mock": "^7.0.0-alpha.6", + "fetch-mock": "^7.7.3", "file-loader": "^6.0.0", "fork-ts-checker-webpack-plugin": "^0.4.9", "ignore-styles": "^5.0.1", @@ -267,6 +267,7 @@ "less": "^3.9.0", "less-loader": "^5.0.0", "mini-css-extract-plugin": "^0.4.0", + "node-fetch": "^2.6.0", "optimize-css-assets-webpack-plugin": "^5.0.1", "po2json": "^0.4.5", "prettier": "^2.0.5", diff --git a/superset-frontend/spec/.eslintrc b/superset-frontend/spec/.eslintrc index 28c59ef..8689bb8 100644 --- a/superset-frontend/spec/.eslintrc +++ b/superset-frontend/spec/.eslintrc @@ -1,14 +1,11 @@ { - "plugins": [ - "jest", - "no-only-tests" - ], + "plugins": ["jest", "no-only-tests"], "env": { "jest/globals": true }, "extends": ["plugin:jest/recommended"], "rules": { - "import/no-extraneous-dependencies": ["error", {"devDependencies": true}], + "import/no-extraneous-dependencies": ["error", { "devDependencies": true }], "jest/consistent-test-it": "error", "no-only-tests/no-only-tests": "error" } diff --git a/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx index 213ac39..d640116 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx @@ -47,6 +47,7 @@ const mockCharts = [...new Array(3)].map((_, i) => ({ url: 'url', viz_type: 'bar', datasource_name: `ds${i}`, + thumbnail_url: '/thumbnail', })); fetchMock.get(chartsInfoEndpoint, { @@ -70,6 +71,9 @@ fetchMock.get(chartsDtasourcesEndpoint, { count: 0, }); +global.URL.createObjectURL = jest.fn(); +fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false }); + describe('ChartList', () => { const mockedProps = {}; const wrapper = mount(<ChartList {...mockedProps} />, { diff --git a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx index 37d5ca2..7eb634c 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx @@ -48,6 +48,7 @@ const mockDashboards = [...new Array(3)].map((_, i) => ({ changed_on_utc: new Date().toISOString(), changed_on_delta_humanized: '5 minutes ago', owners: [{ first_name: 'admin', last_name: 'admin_user' }], + thumbnail_url: '/thumbnail', })); fetchMock.get(dashboardsInfoEndpoint, { @@ -61,6 +62,9 @@ fetchMock.get(dashboardsEndpoint, { dashboard_count: 3, }); +global.URL.createObjectURL = jest.fn(); +fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false }); + describe('DashboardList', () => { const mockedProps = {}; const wrapper = mount(<DashboardList {...mockedProps} />, { diff --git a/superset-frontend/src/common/components/index.js b/superset-frontend/src/common/components/index.ts similarity index 82% rename from superset-frontend/src/common/components/index.js rename to superset-frontend/src/common/components/index.ts index 3b4d245..cb37127 100644 --- a/superset-frontend/src/common/components/index.js +++ b/superset-frontend/src/common/components/index.ts @@ -16,6 +16,8 @@ * specific language governing permissions and limitations * under the License. */ +import styled from '@superset-ui/style'; +import { Skeleton } from 'antd'; /* Antd is exported from here so we can override components with Emotion as needed. @@ -23,4 +25,15 @@ For documentation, see https://ant.design/components/overview/ */ /* eslint no-restricted-imports: 0 */ + export * from 'antd'; + +export const ThinSkeleton = styled(Skeleton)` + h3 { + margin: ${({ theme }) => theme.gridUnit}px 0; + } + + ul { + margin-bottom: 0; + } +`; diff --git a/superset-frontend/src/components/Label/Label.test.tsx b/superset-frontend/src/components/Label/Label.test.tsx index 9227727..1535d4c 100644 --- a/superset-frontend/src/components/Label/Label.test.tsx +++ b/superset-frontend/src/components/Label/Label.test.tsx @@ -17,9 +17,7 @@ * under the License. */ -/* global jest */ import React from 'react'; -/* eslint-disable-next-line import/no-extraneous-dependencies */ import { ReactWrapper } from 'enzyme'; import { styledMount as mount } from 'spec/helpers/theming'; import Label from '.'; diff --git a/superset-frontend/src/components/ListView/CardCollection.tsx b/superset-frontend/src/components/ListView/CardCollection.tsx index 065f8c3..49e4955 100644 --- a/superset-frontend/src/components/ListView/CardCollection.tsx +++ b/superset-frontend/src/components/ListView/CardCollection.tsx @@ -17,8 +17,9 @@ * under the License. */ import React from 'react'; -import { TableInstance } from 'react-table'; +import { TableInstance, Row } from 'react-table'; import styled from '@superset-ui/style'; +import cx from 'classnames'; interface CardCollectionProps { bulkSelectEnabled?: boolean; @@ -42,6 +43,9 @@ const CardWrapper = styled.div` &.card-selected { border: 2px solid ${({ theme }) => theme.colors.primary.base}; } + &.bulk-select { + cursor: pointer; + } `; export default function CardCollection({ @@ -51,32 +55,43 @@ export default function CardCollection({ renderCard, rows, }: CardCollectionProps) { - function handleClick(event: React.FormEvent, onClick: any) { + function handleClick( + event: React.MouseEvent<HTMLDivElement, MouseEvent>, + toggleRowSelected: Row['toggleRowSelected'], + ) { if (bulkSelectEnabled) { event.preventDefault(); event.stopPropagation(); - onClick(); + toggleRowSelected(); } } + if (!renderCard) return null; return ( <CardContainer> - {rows.map(row => { - if (!renderCard) return null; - prepareRow(row); - return ( - <CardWrapper - className={ - row.isSelected && bulkSelectEnabled ? 'card-selected' : '' - } - key={row.id} - onClick={e => handleClick(e, row.toggleRowSelected())} - role="none" - > - {renderCard({ ...row.original, loading })} - </CardWrapper> - ); - })} + {loading && + rows.length === 0 && + [...new Array(25)].map((e, i) => { + return <div key={i}>{renderCard({ loading })}</div>; + })} + {rows.length > 0 && + rows.map(row => { + if (!renderCard) return null; + prepareRow(row); + return ( + <CardWrapper + className={cx({ + 'card-selected': bulkSelectEnabled && row.isSelected, + 'bulk-select': bulkSelectEnabled, + })} + key={row.id} + onClick={e => handleClick(e, row.toggleRowSelected)} + role="none" + > + {renderCard({ ...row.original, loading })} + </CardWrapper> + ); + })} </CardContainer> ); } diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index edcaa61..9e92c77 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -339,11 +339,13 @@ const ListView: FunctionComponent<ListViewProps> = ({ prepareRow={prepareRow} headerGroups={headerGroups} rows={rows} + columns={columns} loading={loading} /> )} </div> </div> + <div className="pagination-container"> <Pagination totalPages={pageCount || 0} @@ -352,12 +354,13 @@ const ListView: FunctionComponent<ListViewProps> = ({ hideFirstAndLastPageLinks /> <div className="row-count-container"> - {t( - '%s-%s of %s', - pageSize * pageIndex + (rows.length && 1), - pageSize * pageIndex + rows.length, - count, - )} + {!loading && + t( + '%s-%s of %s', + pageSize * pageIndex + (rows.length && 1), + pageSize * pageIndex + rows.length, + count, + )} </div> </div> </ListViewStyles> diff --git a/superset-frontend/src/components/ListView/TableCollection.tsx b/superset-frontend/src/components/ListView/TableCollection.tsx index 7fd37f3..71680b8 100644 --- a/superset-frontend/src/components/ListView/TableCollection.tsx +++ b/superset-frontend/src/components/ListView/TableCollection.tsx @@ -28,6 +28,7 @@ interface TableCollectionProps { prepareRow: TableInstance['prepareRow']; headerGroups: TableInstance['headerGroups']; rows: TableInstance['rows']; + columns: TableInstance['column'][]; loading: boolean; } @@ -195,6 +196,7 @@ export default function TableCollection({ getTableBodyProps, prepareRow, headerGroups, + columns, rows, loading, }: TableCollectionProps) { @@ -231,37 +233,60 @@ export default function TableCollection({ ))} </thead> <tbody {...getTableBodyProps()}> - {rows.map(row => { - prepareRow(row); - return ( - <tr - {...row.getRowProps()} - className={cx('table-row', { - 'table-row-selected': row.isSelected, - })} - > - {row.cells.map(cell => { - if (cell.column.hidden) return null; - - const columnCellProps = cell.column.cellProps || {}; + {loading && + rows.length === 0 && + [...new Array(25)].map((_, i) => ( + <tr key={i}> + {columns.map((column, i2) => { + if (column.hidden) return null; return ( <td + key={i2} className={cx('table-cell', { 'table-cell-loader': loading, - [cell.column.size || '']: cell.column.size, + [column.size || '']: column.size, })} - {...cell.getCellProps()} - {...columnCellProps} > - <span className={cx({ 'loading-bar': loading })}> - <span>{cell.render('Cell')}</span> + <span className="loading-bar"> + <span>LOADING</span> </span> </td> ); })} </tr> - ); - })} + ))} + {rows.length > 0 && + rows.map(row => { + prepareRow(row); + return ( + <tr + {...row.getRowProps()} + className={cx('table-row', { + 'table-row-selected': row.isSelected, + })} + > + {row.cells.map(cell => { + if (cell.column.hidden) return null; + + const columnCellProps = cell.column.cellProps || {}; + return ( + <td + className={cx('table-cell', { + 'table-cell-loader': loading, + [cell.column.size || '']: cell.column.size, + })} + {...cell.getCellProps()} + {...columnCellProps} + > + <span className={cx({ 'loading-bar': loading })}> + <span>{cell.render('Cell')}</span> + </span> + </td> + ); + })} + </tr> + ); + })} </tbody> </Table> ); diff --git a/superset-frontend/src/components/ListViewCard/ImageLoader.test.jsx b/superset-frontend/src/components/ListViewCard/ImageLoader.test.jsx new file mode 100644 index 0000000..9d898d4 --- /dev/null +++ b/superset-frontend/src/components/ListViewCard/ImageLoader.test.jsx @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { styledMount as mount } from 'spec/helpers/theming'; +import fetchMock from 'fetch-mock'; + +import ImageLoader from 'src/components/ListViewCard/ImageLoader'; +import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; + +global.URL.createObjectURL = jest.fn(() => '/local_url'); +const blob = new Blob([], { type: 'image/png' }); + +fetchMock.get( + '/thumbnail', + { body: blob, headers: { 'Content-Type': 'image/png' } }, + { + sendAsJson: false, + }, +); + +describe('ListViewCard', () => { + const defaultProps = { + src: '/thumbnail', + fallback: '/fallback', + }; + + const factory = (extraProps = {}) => { + const props = { ...defaultProps, ...extraProps }; + return mount(<ImageLoader {...props} />); + }; + + afterEach(fetchMock.resetHistory); + + it('is a valid element', async () => { + const wrapper = factory(); + await waitForComponentToPaint(wrapper); + expect(wrapper.find(ImageLoader)).toExist(); + }); + + it('fetches loads the image in the background', async () => { + const wrapper = factory(); + expect(wrapper.find('img').props().src).toBe('/fallback'); + await waitForComponentToPaint(wrapper); + expect(fetchMock.calls(/thumbnail/)).toHaveLength(1); + expect(global.URL.createObjectURL).toHaveBeenCalled(); + expect(wrapper.find('img').props().src).toBe('/local_url'); + }); + + it('displays fallback image when response is not an image', async () => { + fetchMock.once('/thumbnail2', {}); + const wrapper = factory({ src: '/thumbnail2' }); + expect(wrapper.find('img').props().src).toBe('/fallback'); + await waitForComponentToPaint(wrapper); + expect(fetchMock.calls(/thumbnail2/)).toHaveLength(1); + expect(wrapper.find('img').props().src).toBe('/fallback'); + }); +}); diff --git a/superset-frontend/src/components/ListViewCard/ImageLoader.tsx b/superset-frontend/src/components/ListViewCard/ImageLoader.tsx new file mode 100644 index 0000000..a4c859e --- /dev/null +++ b/superset-frontend/src/components/ListViewCard/ImageLoader.tsx @@ -0,0 +1,64 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { useEffect } from 'react'; + +interface ImageLoaderProps + extends React.DetailedHTMLProps< + React.ImgHTMLAttributes<HTMLImageElement>, + HTMLImageElement + > { + fallback: string; + src: string; + isLoading: boolean; +} + +export default function ImageLoader({ + src, + fallback, + alt, + isLoading, + ...rest +}: ImageLoaderProps) { + const [imgSrc, setImgSrc] = React.useState<string>(fallback); + + useEffect(() => { + if (src) { + fetch(src) + .then(response => response.blob()) + .then(blob => { + if (/image/.test(blob.type)) { + const imgURL = URL.createObjectURL(blob); + setImgSrc(imgURL); + } + }) + .catch(e => { + console.error(e); // eslint-disable-line no-console + setImgSrc(fallback); + }); + } + + return () => { + // theres a very brief period where isLoading is false and this component is about to unmount + // where the stale imgSrc is briefly rendered. Setting imgSrc to fallback smoothes the transition. + setImgSrc(fallback); + }; + }, [src, fallback]); + + return <img alt={alt || ''} src={isLoading ? fallback : imgSrc} {...rest} />; +} diff --git a/superset-frontend/src/components/ListViewCard/ListViewCard.stories.tsx b/superset-frontend/src/components/ListViewCard/ListViewCard.stories.tsx index 07881f6..c25eeb7 100644 --- a/superset-frontend/src/components/ListViewCard/ListViewCard.stories.tsx +++ b/superset-frontend/src/components/ListViewCard/ListViewCard.stories.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; import { action } from '@storybook/addon-actions'; -import { withKnobs, boolean } from '@storybook/addon-knobs'; +import { withKnobs, boolean, select, text } from '@storybook/addon-knobs'; import DashboardImg from 'images/dashboard-card-fallback.png'; import ChartImg from 'images/chart-card-fallback.png'; import { Dropdown, Menu } from 'src/common/components'; @@ -32,13 +32,27 @@ export default { decorators: [withKnobs], }; +const imgFallbackKnob = { + label: 'Fallback/Loading Image', + options: { + Dashboard: DashboardImg, + Chart: ChartImg, + }, + defaultValue: DashboardImg, +}; + export const SupersetListViewCard = () => { return ( <ListViewCard title="Superset Card Title" + loading={boolean('loading', false)} url="/superset/dashboard/births/" - imgURL={DashboardImg} - imgFallbackURL={ChartImg} + imgURL={text('imgURL', 'https://picsum.photos/800/600')} + imgFallbackURL={select( + imgFallbackKnob.label, + imgFallbackKnob.options, + imgFallbackKnob.defaultValue, + )} description="Lorem ipsum dolor sit amet, consectetur adipiscing elit..." coverLeft="Left Section" coverRight="Right Section" diff --git a/superset-frontend/src/components/ListViewCard/ListViewCard.test.jsx b/superset-frontend/src/components/ListViewCard/ListViewCard.test.jsx new file mode 100644 index 0000000..8387657 --- /dev/null +++ b/superset-frontend/src/components/ListViewCard/ListViewCard.test.jsx @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { styledMount as mount } from 'spec/helpers/theming'; +import fetchMock from 'fetch-mock'; + +import ListViewCard from 'src/components/ListViewCard'; +import ImageLoader from 'src/components/ListViewCard/ImageLoader'; +import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; + +global.URL.createObjectURL = jest.fn(() => '/local_url'); +fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false }); + +describe('ListViewCard', () => { + const defaultProps = { + title: 'Card Title', + loading: false, + url: '/card-url', + imgURL: '/thumbnail', + imgFallbackURL: '/fallback', + description: 'Card Description', + coverLeft: 'Left Text', + coverRight: 'Right Text', + actions: ( + <ListViewCard.Actions> + <div>Action 1</div> + <div>Action 2</div> + </ListViewCard.Actions> + ), + }; + + let wrapper; + const factory = (extraProps = {}) => { + const props = { ...defaultProps, ...extraProps }; + return mount(<ListViewCard {...props} />); + }; + beforeEach(async () => { + wrapper = factory(); + await waitForComponentToPaint(wrapper); + }); + + it('is a valid element', () => { + expect(wrapper.find(ListViewCard)).toExist(); + }); + + it('renders Actions', () => { + expect(wrapper.find(ListViewCard.Actions)).toExist(); + }); + + it('renders and ImageLoader', () => { + expect(wrapper.find(ImageLoader)).toExist(); + }); +}); diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx index f6c5af8..a668487 100644 --- a/superset-frontend/src/components/ListViewCard/index.tsx +++ b/superset-frontend/src/components/ListViewCard/index.tsx @@ -19,7 +19,8 @@ import React from 'react'; import styled from '@superset-ui/style'; import Icon from 'src/components/Icon'; -import { Card } from 'src/common/components'; +import { Card, Skeleton, ThinSkeleton } from 'src/common/components'; +import ImageLoader from './ImageLoader'; const MenuIcon = styled(Icon)` width: ${({ theme }) => theme.gridUnit * 4}px; @@ -82,7 +83,7 @@ const GradientContainer = styled.div` ); } `; -const CardCoverImg = styled.img` +const CardCoverImg = styled(ImageLoader)` display: block; object-fit: cover; width: 459px; @@ -132,12 +133,22 @@ const CoverFooterRight = styled.div` text-overflow: ellipsis; `; +const SkeletonTitle = styled(Skeleton.Input)` + width: ${({ theme }) => Math.trunc(theme.gridUnit * 62.5)}px; +`; + +const SkeletonActions = styled(Skeleton.Button)` + width: ${({ theme }) => theme.gridUnit * 10}px; +`; + +const paragraphConfig = { rows: 1, width: 150 }; interface CardProps { title: React.ReactNode; - url: string | undefined; + url?: string; imgURL: string; imgFallbackURL: string; description: string; + loading: boolean; titleRight?: React.ReactNode; coverLeft?: React.ReactNode; coverRight?: React.ReactNode; @@ -154,6 +165,7 @@ function ListViewCard({ coverLeft, coverRight, actions, + loading, }: CardProps) { return ( <StyledCard @@ -163,31 +175,59 @@ function ListViewCard({ <GradientContainer> <CardCoverImg src={imgURL} - onError={e => { - e.currentTarget.src = imgFallbackURL; - }} + fallback={imgFallbackURL} + isLoading={loading} /> </GradientContainer> </a> <CoverFooter className="cover-footer"> - {coverLeft && <CoverFooterLeft>{coverLeft}</CoverFooterLeft>} - {coverRight && <CoverFooterRight>{coverRight}</CoverFooterRight>} + {!loading && coverLeft && ( + <CoverFooterLeft>{coverLeft}</CoverFooterLeft> + )} + {!loading && coverRight && ( + <CoverFooterRight>{coverRight}</CoverFooterRight> + )} </CoverFooter> </Cover> } > - <Card.Meta - title={ - <> - <TitleContainer> - <TitleLink href={url}>{title}</TitleLink> - {titleRight && <div className="title-right"> {titleRight}</div>} - <div className="card-actions">{actions}</div> - </TitleContainer> - </> - } - description={description} - /> + {loading && ( + <Card.Meta + title={ + <> + <TitleContainer> + <SkeletonTitle active size="small" /> + <div className="card-actions"> + <Skeleton.Button active shape="circle" />{' '} + <SkeletonActions active /> + </div> + </TitleContainer> + </> + } + description={ + <ThinSkeleton + round + active + title={false} + paragraph={paragraphConfig} + /> + } + /> + )} + {!loading && ( + <Card.Meta + title={ + <> + <TitleContainer> + <TitleLink href={url}>{title}</TitleLink> + {titleRight && <div className="title-right"> {titleRight}</div>} + <div className="card-actions">{actions}</div> + </TitleContainer> + </> + } + description={description} + /> + )} </StyledCard> ); } diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 5b2de1b..fb8d905 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -482,7 +482,7 @@ class ChartList extends React.PureComponent<Props, State> { }); }; - renderCard = (props: Chart) => { + renderCard = (props: Chart & { loading: boolean }) => { const menu = ( <Menu> {this.canDelete && ( @@ -524,6 +524,7 @@ class ChartList extends React.PureComponent<Props, State> { return ( <ListViewCard + loading={props.loading} title={props.slice_name} url={this.state.bulkSelectEnabled ? undefined : props.url} imgURL={props.thumbnail_url ?? ''} diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index 6e6e467..437edbb 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -477,7 +477,7 @@ class DashboardList extends React.PureComponent<Props, State> { ); } - renderCard = (props: Dashboard) => { + renderCard = (props: Dashboard & { loading: boolean }) => { const menu = ( <Menu> {this.canDelete && ( @@ -529,12 +529,13 @@ class DashboardList extends React.PureComponent<Props, State> { return ( <ListViewCard title={props.dashboard_title} + loading={props.loading} titleRight={<Label>{props.published ? 'published' : 'draft'}</Label>} url={this.state.bulkSelectEnabled ? undefined : props.url} imgURL={props.thumbnail_url} imgFallbackURL="/static/assets/images/dashboard-card-fallback.png" description={t('Last modified %s', props.changed_on_delta_humanized)} - coverLeft={props.owners.slice(0, 5).map(owner => ( + coverLeft={(props.owners || []).slice(0, 5).map(owner => ( <AvatarIcon key={owner.id} uniqueKey={`${owner.username}-${props.id}`}
