From 81525c3e9d03e14def5477ab1902a23049769ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CA=88=E1=B5=83=E1=B5=A2?= Date: Thu, 27 Aug 2020 09:40:32 -0700 Subject: [PATCH] feat(listview): set default view mode based on THUMBNAIL feature flag (#10691) * feat(listview): set default view mode based on THUMBNAIL feature flag * add spec * better generic typing for ListView * lint * fix specs --- .../components/ListView/ListView_spec.jsx | 24 +++++++++++++++++-- .../views/CRUD/chart/ChartList_spec.jsx | 8 +++++++ .../CRUD/dashboard/DashboardList_spec.jsx | 9 +++++++ .../src/components/ListView/ListView.tsx | 20 +++++++++------- superset-frontend/src/featureFlags.ts | 1 + .../src/views/CRUD/chart/ChartList.tsx | 6 ++++- .../views/CRUD/dashboard/DashboardList.tsx | 6 ++++- .../views/CRUD/data/dataset/DatasetList.tsx | 2 +- 8 files changed, 63 insertions(+), 13 deletions(-) diff --git a/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx b/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx index fee729e11..a60f5ed4e 100644 --- a/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx +++ b/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx @@ -292,13 +292,13 @@ describe('ListView', () => { ); }); - it('disable card view based on prop', async () => { + it('disables card view based on prop', async () => { expect(wrapper.find(CardCollection).exists()).toBe(false); expect(wrapper.find(CardSortSelect).exists()).toBe(false); expect(wrapper.find(TableCollection).exists()).toBe(true); }); - it('enable card view based on prop', async () => { + it('enables card view based on prop', async () => { const wrapper2 = factory({ ...mockedProps, renderCard: jest.fn(), @@ -310,6 +310,26 @@ describe('ListView', () => { expect(wrapper2.find(TableCollection).exists()).toBe(false); }); + it('allows setting the default view mode', async () => { + const wrapper2 = factory({ + ...mockedProps, + renderCard: jest.fn(), + defaultViewMode: 'card', + initialSort: [{ id: 'something' }], + }); + await waitForComponentToPaint(wrapper2); + expect(wrapper2.find(CardCollection).exists()).toBe(true); + + const wrapper3 = factory({ + ...mockedProps, + renderCard: jest.fn(), + defaultViewMode: 'table', + initialSort: [{ id: 'something' }], + }); + await waitForComponentToPaint(wrapper3); + expect(wrapper3.find(TableCollection).exists()).toBe(true); + }); + it('Throws an exception if filter missing in columns', () => { expect.assertions(1); const props = { 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 73dd9eea3..14366a444 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx @@ -20,6 +20,7 @@ import React from 'react'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; +import * as featureFlags from 'src/featureFlags'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; import { styledMount as mount } from 'spec/helpers/theming'; @@ -75,6 +76,13 @@ global.URL.createObjectURL = jest.fn(); fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false }); describe('ChartList', () => { + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation(feature => feature === 'THUMBNAILS'); + + afterAll(() => { + isFeatureEnabledMock.restore(); + }); const mockedProps = {}; const wrapper = mount(, { context: { store }, 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 fd824147f..f5c18d3a3 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx @@ -20,6 +20,7 @@ import React from 'react'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; +import * as featureFlags from 'src/featureFlags'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; import { styledMount as mount } from 'spec/helpers/theming'; @@ -67,6 +68,14 @@ global.URL.createObjectURL = jest.fn(); fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false }); describe('DashboardList', () => { + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation(feature => feature === 'THUMBNAILS'); + + afterAll(() => { + isFeatureEnabledMock.restore(); + }); + const mockedProps = {}; const wrapper = mount(, { context: { store }, diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index 9e92c7797..f825753a0 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -17,7 +17,7 @@ * under the License. */ import { t } from '@superset-ui/translation'; -import React, { FunctionComponent, useState } from 'react'; +import React, { useState } from 'react'; import { Alert } from 'react-bootstrap'; import styled from '@superset-ui/style'; import cx from 'classnames'; @@ -174,7 +174,9 @@ const ViewModeToggle = ({ ); }; -export interface ListViewProps { + +type ViewModeType = 'card' | 'table'; +export interface ListViewProps { columns: any[]; data: T[]; count: number; @@ -193,11 +195,12 @@ export interface ListViewProps { bulkSelectEnabled?: boolean; disableBulkSelect?: () => void; renderBulkSelectCopy?: (selects: any[]) => React.ReactNode; - renderCard?: (row: T) => React.ReactNode; + renderCard?: (row: T & { loading: boolean }) => React.ReactNode; cardSortSelectOptions?: Array; + defaultViewMode?: ViewModeType; } -const ListView: FunctionComponent = ({ +function ListView({ columns, data, count, @@ -213,7 +216,8 @@ const ListView: FunctionComponent = ({ renderBulkSelectCopy = selected => t('%s Selected', selected.length), renderCard, cardSortSelectOptions, -}) => { + defaultViewMode = 'card', +}: ListViewProps) { const { getTableProps, getTableBodyProps, @@ -253,8 +257,8 @@ const ListView: FunctionComponent = ({ } const cardViewEnabled = Boolean(renderCard); - const [viewingMode, setViewingMode] = useState<'table' | 'card'>( - cardViewEnabled ? 'card' : 'table', + const [viewingMode, setViewingMode] = useState( + cardViewEnabled ? defaultViewMode : 'table', ); return ( @@ -365,6 +369,6 @@ const ListView: FunctionComponent = ({ ); -}; +} export default ListView; diff --git a/superset-frontend/src/featureFlags.ts b/superset-frontend/src/featureFlags.ts index 08535787d..35817c74b 100644 --- a/superset-frontend/src/featureFlags.ts +++ b/superset-frontend/src/featureFlags.ts @@ -26,6 +26,7 @@ export enum FeatureFlag { ESTIMATE_QUERY_COST = 'ESTIMATE_QUERY_COST', SHARE_QUERIES_VIA_KV_STORE = 'SHARE_QUERIES_VIA_KV_STORE', SQLLAB_BACKEND_PERSISTENCE = 'SQLLAB_BACKEND_PERSISTENCE', + THUMBNAILS = 'THUMBNAILS', } export type FeatureFlagMap = { diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 514a93a70..bf2fded8b 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -22,6 +22,7 @@ import { getChartMetadataRegistry } from '@superset-ui/chart'; import React, { useState, useMemo } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; +import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import { useListViewResource, useFavoriteStatus } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; @@ -502,7 +503,7 @@ function ChartList(props: ChartListProps) { : []; return ( - bulkActions={bulkActions} bulkSelectEnabled={bulkSelectEnabled} cardSortSelectOptions={sortTypes} @@ -517,6 +518,9 @@ function ChartList(props: ChartListProps) { loading={loading} pageSize={PAGE_SIZE} renderCard={renderCard} + defaultViewMode={ + isFeatureEnabled(FeatureFlag.THUMBNAILS) ? 'card' : 'table' + } /> ); }} diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index fded93a1b..29f40b3e5 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -20,6 +20,7 @@ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; import React, { useState, useMemo } from 'react'; import rison from 'rison'; +import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import { useListViewResource, useFavoriteStatus } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; @@ -510,7 +511,7 @@ function DashboardList(props: DashboardListProps) { onSubmit={handleDashboardEdit} /> )} - bulkActions={bulkActions} bulkSelectEnabled={bulkSelectEnabled} cardSortSelectOptions={sortTypes} @@ -525,6 +526,9 @@ function DashboardList(props: DashboardListProps) { loading={loading} pageSize={PAGE_SIZE} renderCard={renderCard} + defaultViewMode={ + isFeatureEnabled(FeatureFlag.THUMBNAILS) ? 'card' : 'table' + } /> ); diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 668739c5f..c11bac5c9 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -540,7 +540,7 @@ const DatasetList: FunctionComponent = ({ : []; return ( - className="dataset-list-view" columns={columns} data={datasets}