etr2460 commented on a change in pull request #10094:
URL:
https://github.com/apache/incubator-superset/pull/10094#discussion_r442634389
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -116,124 +118,117 @@ const ListView: FunctionComponent<Props> = ({
}
});
}
-
+ if (loading && !data.length) {
+ return <Loading />;
+ }
return (
- <div className={`superset-list-view ${className}`}>
- <div className="header">
- {!useNewUIFilters && (
- <>
- {title && filterable && (
- <>
- <Row>
- <Col md={11}>
- <h2>{t(title)}</h2>
- </Col>
- {filterable && (
- <Col md={1}>
- <FilterMenu
- filters={filters}
- internalFilters={internalFilters}
- setInternalFilters={setInternalFilters}
- />
- </Col>
- )}
- </Row>
- <hr />
- <FilterInputs
- internalFilters={internalFilters}
- filters={filters}
- updateInternalFilter={updateInternalFilter}
- removeFilterAndApply={removeFilterAndApply}
- filtersApplied={filtersApplied}
- applyFilters={applyFilters}
- />
- </>
- )}
- </>
- )}
- {useNewUIFilters && (
- <>
- <Row>
- <Col md={10}>
- <h2>{t(title)}</h2>
- </Col>
- </Row>
- <hr />
+ <div className="superset-list-view-container">
+ <div className={`superset-list-view ${className}`}>
+ <div className="header">
+ {!useNewUIFilters && (
+ <>
Review comment:
do you need this fragment here? in fact, it seems like some extra
nesting with !useNewUIFilers, title, and filterable can be joined into a single
statement
##########
File path: superset-frontend/src/components/ListView/ListViewStyles.less
##########
@@ -19,92 +19,127 @@
@import '~stylesheets/less/variables.less';
-.superset-list-view {
- .filter-dropdown {
- margin-top: 20px;
- }
+.superset-list-view-container {
+ text-align: center;
+
+ .superset-list-view {
+ text-align: left;
+ background-color: white;
+ border-radius: 4px 0;
+ overflow: scroll;
+ margin: 0 16px;
+ padding-bottom: 48px;
+
+ .filter-dropdown {
+ margin-top: 20px;
+ }
- .filter-column {
- height: 30px;
- padding: 5px;
- font-size: 16px;
- }
+ .filter-column {
+ height: 30px;
+ padding: 5px;
+ font-size: 16px;
+ }
- .filter-close {
- height: 30px;
- padding: 5px;
+ .filter-close {
+ height: 30px;
+ padding: 5px;
- i {
- font-size: 20px;
+ i {
+ font-size: 20px;
+ }
}
- }
- .table-row-loader {
- animation: shimmer 2s infinite;
- background: linear-gradient(
- to right,
- #f6f7f8 0%,
- #edeef1 20%,
- #f6f7f8 40%,
- #f6f7f8 100%
- );
- background-size: 1000px 100%;
-
- span {
- visibility: hidden;
+ .table-cell-loader {
+ position: relative;
+
+ .loading-bar {
+ background-color: #e7e7e7;
Review comment:
do we have a variable for this one?
##########
File path: superset-frontend/src/views/chartList/ChartList.tsx
##########
@@ -517,58 +519,55 @@ class ChartList extends React.PureComponent<Props, State>
{
sliceCurrentlyEditing,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- {sliceCurrentlyEditing && (
- <PropertiesModal
- show
- onHide={this.closeChartEditModal}
- onSave={this.handleChartUpdated}
- slice={sliceCurrentlyEditing}
+ <>
+ <SubMenu name="Charts" />
+ {sliceCurrentlyEditing && (
+ <PropertiesModal
+ show
+ onHide={this.closeChartEditModal}
+ onSave={this.handleChartUpdated}
+ slice={sliceCurrentlyEditing}
+ />
+ )}
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={t(
+ 'Are you sure you want to delete the selected charts?',
+ )}
+ onConfirm={this.handleBulkChartDelete}
+ >
+ {confirmDelete => {
+ const bulkActions = [];
+ if (this.canDelete) {
+ bulkActions.push({
+ key: 'delete',
+ name: (
+ <>
+ <i className="fa fa-trash" /> Delete
+ </>
+ ),
+ onSelect: confirmDelete,
+ });
+ }
+ return (
+ <ListView
+ className="chart-list-view"
+ title={'Charts'}
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/types/react-table-config.d.ts
##########
@@ -118,13 +118,19 @@ declare module 'react-table' {
hidden?: boolean;
sortable?: boolean;
cellProps?: any;
+ size?: 'xs' | 'sm' | 'md' | 'lg' | 'xl';
Review comment:
Let's define these strings as it's own `Size` type then reuse it in both
places
##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -508,71 +510,68 @@ class DashboardList extends React.PureComponent<Props,
State> {
dashboardToEdit,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- <ConfirmStatusChange
- title={t('Please confirm')}
- description={t(
- 'Are you sure you want to delete the selected dashboards?',
- )}
- onConfirm={this.handleBulkDashboardDelete}
- >
- {confirmDelete => {
- const bulkActions = [];
- if (this.canDelete) {
- bulkActions.push({
- key: 'delete',
- name: (
- <>
- <i className="fa fa-trash" /> Delete
- </>
- ),
- onSelect: confirmDelete,
- });
- }
- if (this.canExport) {
- bulkActions.push({
- key: 'export',
- name: (
- <>
- <i className="fa fa-database" /> Export
- </>
- ),
- onSelect: this.handleBulkDashboardExport,
- });
- }
- return (
+ <>
Review comment:
it seems like a lot of code is the same as in the above file, is there
any way we can pull it out into its own component and reuse it?
##########
File path: superset-frontend/src/views/datasetList/DatasetList.tsx
##########
@@ -487,49 +524,43 @@ class DatasetList extends React.PureComponent<Props,
State> {
return (
<>
<SubMenu {...this.menu} canCreate={this.canCreate} />
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- <ConfirmStatusChange
- title={t('Please confirm')}
- description={t(
- 'Are you sure you want to delete the selected datasets?',
- )}
- onConfirm={this.handleBulkDatasetDelete}
- >
- {confirmDelete => {
- const bulkActions = [];
- if (this.canDelete) {
- bulkActions.push({
- key: 'delete',
- name: (
- <>
- <i className="fa fa-trash" /> Delete
- </>
- ),
- onSelect: confirmDelete,
- });
- }
- return (
- <ListView
- className="dataset-list-view"
- title={'Datasets'}
- columns={this.columns}
- data={datasets}
- count={datasetCount}
- pageSize={PAGE_SIZE}
- fetchData={this.fetchData}
- loading={loading}
- initialSort={this.initialSort}
- filters={filters}
- bulkActions={bulkActions}
- />
- );
- }}
- </ConfirmStatusChange>
- </Panel.Body>
- </Panel>
- </div>
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={t(
+ 'Are you sure you want to delete the selected datasets?',
+ )}
+ onConfirm={this.handleBulkDatasetDelete}
+ >
+ {confirmDelete => {
+ const bulkActions = [];
+ if (this.canDelete) {
+ bulkActions.push({
+ key: 'delete',
+ name: (
+ <>
+ <i className="fa fa-trash" /> Delete
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -116,124 +118,117 @@ const ListView: FunctionComponent<Props> = ({
}
});
}
-
+ if (loading && !data.length) {
+ return <Loading />;
+ }
return (
- <div className={`superset-list-view ${className}`}>
- <div className="header">
- {!useNewUIFilters && (
- <>
- {title && filterable && (
- <>
- <Row>
- <Col md={11}>
- <h2>{t(title)}</h2>
- </Col>
- {filterable && (
- <Col md={1}>
- <FilterMenu
- filters={filters}
- internalFilters={internalFilters}
- setInternalFilters={setInternalFilters}
- />
- </Col>
- )}
- </Row>
- <hr />
- <FilterInputs
- internalFilters={internalFilters}
- filters={filters}
- updateInternalFilter={updateInternalFilter}
- removeFilterAndApply={removeFilterAndApply}
- filtersApplied={filtersApplied}
- applyFilters={applyFilters}
- />
- </>
- )}
- </>
- )}
- {useNewUIFilters && (
- <>
- <Row>
- <Col md={10}>
- <h2>{t(title)}</h2>
- </Col>
- </Row>
- <hr />
+ <div className="superset-list-view-container">
+ <div className={`superset-list-view ${className}`}>
+ <div className="header">
+ {!useNewUIFilters && (
+ <>
+ {title && filterable && (
+ <>
+ <Row>
+ <Col md={10} />
+ {filterable && (
+ <Col md={2}>
+ <FilterMenu
+ filters={filters}
+ internalFilters={internalFilters}
+ setInternalFilters={setInternalFilters}
+ />
+ </Col>
+ )}
+ </Row>
+ <hr />
+ <FilterInputs
+ internalFilters={internalFilters}
+ filters={filters}
+ updateInternalFilter={updateInternalFilter}
+ removeFilterAndApply={removeFilterAndApply}
+ filtersApplied={filtersApplied}
+ applyFilters={applyFilters}
+ />
+ </>
+ )}
+ </>
+ )}
+ {useNewUIFilters && (
<FilterControls
filters={filters}
internalFilters={internalFilters}
updateFilterValue={applyFilterValue}
/>
- </>
- )}
- </div>
- <div className="body">
- <TableCollection
- getTableProps={getTableProps}
- getTableBodyProps={getTableBodyProps}
- prepareRow={prepareRow}
- headerGroups={headerGroups}
- rows={rows}
- loading={loading}
- />
- </div>
- <div className="footer">
- <Row>
- <Col md={2}>
- <div className="form-actions-container">
- <div className="btn-group">
- {bulkActions.length > 0 && (
- <DropdownButton
- id="bulk-actions"
- bsSize="small"
- bsStyle="default"
- noCaret
- title={
- <>
- {t('Actions')} <span className="caret" />
- </>
- }
- >
- {bulkActions.map(action => (
- // @ts-ignore
- <MenuItem
- key={action.key}
- eventKey={selectedFlatRows}
+ )}
+ </div>
+ <div className="body">
+ <TableCollection
+ getTableProps={getTableProps}
+ getTableBodyProps={getTableBodyProps}
+ prepareRow={prepareRow}
+ headerGroups={headerGroups}
+ rows={rows}
+ loading={loading}
+ />
+ </div>
+ <div className="footer">
+ <Row>
+ <Col>
+ <div className="form-actions-container">
+ <div className="btn-group">
+ {bulkActions.length > 0 && (
+ <DropdownButton
+ id="bulk-actions"
+ bsSize="small"
+ bsStyle="default"
+ noCaret
+ title={
+ <>
+ {t('Actions')} <span className="caret" />
+ </>
+ }
+ >
+ {bulkActions.map(action => (
// @ts-ignore
- onSelect={(selectedRows: typeof selectedFlatRows) => {
- action.onSelect(
- selectedRows.map((r: any) => r.original),
- );
- }}
- >
- {action.name}
- </MenuItem>
- ))}
- </DropdownButton>
- )}
+ <MenuItem
+ key={action.key}
+ eventKey={selectedFlatRows}
+ // @ts-ignore
+ onSelect={(selectedRows: typeof selectedFlatRows) =>
{
+ action.onSelect(
+ selectedRows.map((r: any) => r.original),
+ );
+ }}
+ >
+ {action.name}
+ </MenuItem>
+ ))}
+ </DropdownButton>
+ )}
+ </div>
</div>
- </div>
- </Col>
- <Col md={8} className="text-center">
- <Pagination
- totalPages={pageCount || 0}
- currentPage={pageCount ? pageIndex + 1 : 0}
- onChange={(p: number) => gotoPage(p - 1)}
- hideFirstAndLastPageLinks
- />
- </Col>
- <Col md={2}>
- <span className="pull-right">
- {t('showing')}{' '}
- <strong>
- {pageSize * pageIndex + (rows.length && 1)}-
- {pageSize * pageIndex + rows.length}
- </strong>{' '}
- {t('of')} <strong>{count}</strong>
- </span>
- </Col>
- </Row>
+ </Col>
+
+ <Col>
+ <span className="row-count-container">
+ {t('showing')}{' '}
+ <strong>
+ {pageSize * pageIndex + (rows.length && 1)}-
+ {pageSize * pageIndex + rows.length}
+ </strong>{' '}
+ {t('of')} <strong>{count}</strong>
Review comment:
these translation strings won't work at all. I think using string
replacement is required instead, details are here:
https://github.com/apache-superset/superset-ui/tree/master/packages/superset-ui-translation
although, now that i look closer, it seems like these are just indentation
changes. so i guess it's up to you if you want to fix them or not. not sure how
they snuck past in the first place :/ if it's not in scope to fix them here,
let me know and i can clean up afterwards
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -29,6 +30,18 @@ interface Props {
rows: TableInstance['rows'];
loading: boolean;
}
+
+const TableCell = styled.td`
+ width: ${(props: { size?: Column['size'] }) => {
+ if (props.size === 'xs') return '25px';
+ if (props.size === 'sm') return '50px';
+ if (props.size === 'md') return '75px';
+ if (props.size === 'lg') return '100px';
+ if (props.size === 'xl') return '150px';
+ if (props.size === 'xxl') return '200px';
+ return '';
+ }};
Review comment:
this could be simpler if you have each if only return a number, then you
append `px` on at the end. Note the final return would need to return 0 instead
of empty string then.
on a side note, do you think a switch statement is better here? it would
remove even more code duplication
##########
File path: superset-frontend/src/components/ListView/ListViewStyles.less
##########
@@ -19,92 +19,127 @@
@import '~stylesheets/less/variables.less';
-.superset-list-view {
- .filter-dropdown {
- margin-top: 20px;
- }
+.superset-list-view-container {
+ text-align: center;
+
+ .superset-list-view {
+ text-align: left;
+ background-color: white;
+ border-radius: 4px 0;
+ overflow: scroll;
+ margin: 0 16px;
+ padding-bottom: 48px;
+
+ .filter-dropdown {
+ margin-top: 20px;
+ }
- .filter-column {
- height: 30px;
- padding: 5px;
- font-size: 16px;
- }
+ .filter-column {
+ height: 30px;
+ padding: 5px;
+ font-size: 16px;
+ }
- .filter-close {
- height: 30px;
- padding: 5px;
+ .filter-close {
+ height: 30px;
+ padding: 5px;
- i {
- font-size: 20px;
+ i {
+ font-size: 20px;
+ }
}
- }
- .table-row-loader {
- animation: shimmer 2s infinite;
- background: linear-gradient(
- to right,
- #f6f7f8 0%,
- #edeef1 20%,
- #f6f7f8 40%,
- #f6f7f8 100%
- );
- background-size: 1000px 100%;
-
- span {
- visibility: hidden;
+ .table-cell-loader {
+ position: relative;
+
+ .loading-bar {
+ background-color: #e7e7e7;
+ border-radius: 7px;
+
+ span {
+ visibility: hidden;
+ }
+ }
+
+ &:after {
+ position: absolute;
+ transform: translateY(-50%);
+ top: 50%;
+ left: 0;
+ content: '';
+ display: block;
+ width: 100%;
+ height: 48px;
+ background-image: linear-gradient(
+ 100deg,
+ rgba(255, 255, 255, 0),
+ rgba(255, 255, 255, 0.5) 60%,
+ rgba(255, 255, 255, 0) 80%
Review comment:
feels like we might have variables for these colors already, although
i'm not sure. Maybe we should add them if not?
##########
File path: superset-frontend/src/components/Pagination.tsx
##########
@@ -0,0 +1,132 @@
+/**
+ * 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, { PureComponent } from 'react';
+import cx from 'classnames';
+import styled from '@superset-ui/style';
+
+interface PaginationButton {
+ disabled?: boolean;
+ onClick: React.EventHandler<React.SyntheticEvent<HTMLElement>>;
+}
+
+interface PaginationItemButton extends PaginationButton {
+ active: boolean;
+ children: React.ReactNode;
+}
+
+function Prev({ disabled, onClick }: PaginationButton) {
+ return (
+ <li className={cx({ disabled })}>
+ <span role="button" tabIndex={disabled ? -1 : 0} onClick={onClick}>
+ «
Review comment:
is using unicode a good idea for this, or should we add svg icons while
we're in here making stuff closer to SIP-34?
##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -508,71 +510,68 @@ class DashboardList extends React.PureComponent<Props,
State> {
dashboardToEdit,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- <ConfirmStatusChange
- title={t('Please confirm')}
- description={t(
- 'Are you sure you want to delete the selected dashboards?',
- )}
- onConfirm={this.handleBulkDashboardDelete}
- >
- {confirmDelete => {
- const bulkActions = [];
- if (this.canDelete) {
- bulkActions.push({
- key: 'delete',
- name: (
- <>
- <i className="fa fa-trash" /> Delete
- </>
- ),
- onSelect: confirmDelete,
- });
- }
- if (this.canExport) {
- bulkActions.push({
- key: 'export',
- name: (
- <>
- <i className="fa fa-database" /> Export
- </>
- ),
- onSelect: this.handleBulkDashboardExport,
- });
- }
- return (
+ <>
+ <SubMenu name="Dashboards" />
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={t(
+ 'Are you sure you want to delete the selected dashboards?',
+ )}
+ onConfirm={this.handleBulkDashboardDelete}
+ >
+ {confirmDelete => {
+ const bulkActions = [];
+ if (this.canDelete) {
+ bulkActions.push({
+ key: 'delete',
+ name: (
+ <>
+ <i className="fa fa-trash" /> Delete
+ </>
+ ),
+ onSelect: confirmDelete,
+ });
+ }
+ if (this.canExport) {
+ bulkActions.push({
+ key: 'export',
+ name: (
<>
- {dashboardToEdit && (
- <PropertiesModal
- show
- dashboardId={dashboardToEdit.id}
- onHide={() => this.setState({ dashboardToEdit: null })}
- onDashboardSave={this.handleDashboardEdit}
- />
- )}
- <ListView
- className="dashboard-list-view"
- title={'Dashboards'}
- columns={this.columns}
- data={dashboards}
- count={dashboardCount}
- pageSize={PAGE_SIZE}
- fetchData={this.fetchData}
- loading={loading}
- initialSort={this.initialSort}
- filters={filters}
- bulkActions={bulkActions}
- useNewUIFilters={this.isNewUIEnabled}
- />
+ <i className="fa fa-database" /> Export
</>
- );
- }}
- </ConfirmStatusChange>
- </Panel.Body>
- </Panel>
- </div>
+ ),
+ onSelect: this.handleBulkDashboardExport,
+ });
+ }
+ return (
+ <>
+ {dashboardToEdit && (
+ <PropertiesModal
+ show
+ dashboardId={dashboardToEdit.id}
+ onHide={() => this.setState({ dashboardToEdit: null })}
+ onDashboardSave={this.handleDashboardEdit}
+ />
+ )}
+ <ListView
+ className="dashboard-list-view"
+ title={'Dashboards'}
Review comment:
nit, `t` string here please!
##########
File path: superset-frontend/src/components/Pagination.tsx
##########
@@ -0,0 +1,132 @@
+/**
+ * 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, { PureComponent } from 'react';
+import cx from 'classnames';
+import styled from '@superset-ui/style';
+
+interface PaginationButton {
+ disabled?: boolean;
+ onClick: React.EventHandler<React.SyntheticEvent<HTMLElement>>;
+}
+
+interface PaginationItemButton extends PaginationButton {
+ active: boolean;
+ children: React.ReactNode;
+}
+
+function Prev({ disabled, onClick }: PaginationButton) {
+ return (
+ <li className={cx({ disabled })}>
+ <span role="button" tabIndex={disabled ? -1 : 0} onClick={onClick}>
+ «
+ </span>
+ </li>
+ );
+}
+
+function Next({ disabled, onClick }: PaginationButton) {
+ return (
+ <li className={cx({ disabled })}>
+ <span role="button" tabIndex={disabled ? -1 : 0} onClick={onClick}>
+ »
+ </span>
+ </li>
+ );
+}
+
+function Item({ active, children, onClick }: PaginationItemButton) {
+ return (
+ <li className={cx({ active })}>
+ <span role="button" tabIndex={active ? -1 : 0} onClick={onClick}>
+ {children}
+ </span>
+ </li>
+ );
+}
+
+function Ellipsis({ disabled, onClick }: PaginationButton) {
+ return (
+ <li className={cx({ disabled })}>
+ <span role="button" tabIndex={disabled ? -1 : 0} onClick={onClick}>
+ ...
Review comment:
should this be `...` or `…` (3 periods vs. an ellipsis)?
##########
File path: superset-frontend/src/views/datasetList/DatasetList.tsx
##########
@@ -487,49 +524,43 @@ class DatasetList extends React.PureComponent<Props,
State> {
return (
<>
<SubMenu {...this.menu} canCreate={this.canCreate} />
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- <ConfirmStatusChange
- title={t('Please confirm')}
- description={t(
- 'Are you sure you want to delete the selected datasets?',
- )}
- onConfirm={this.handleBulkDatasetDelete}
- >
- {confirmDelete => {
- const bulkActions = [];
- if (this.canDelete) {
- bulkActions.push({
- key: 'delete',
- name: (
- <>
- <i className="fa fa-trash" /> Delete
- </>
- ),
- onSelect: confirmDelete,
- });
- }
- return (
- <ListView
- className="dataset-list-view"
- title={'Datasets'}
- columns={this.columns}
- data={datasets}
- count={datasetCount}
- pageSize={PAGE_SIZE}
- fetchData={this.fetchData}
- loading={loading}
- initialSort={this.initialSort}
- filters={filters}
- bulkActions={bulkActions}
- />
- );
- }}
- </ConfirmStatusChange>
- </Panel.Body>
- </Panel>
- </div>
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={t(
+ 'Are you sure you want to delete the selected datasets?',
+ )}
+ onConfirm={this.handleBulkDatasetDelete}
+ >
+ {confirmDelete => {
+ const bulkActions = [];
+ if (this.canDelete) {
+ bulkActions.push({
+ key: 'delete',
+ name: (
+ <>
+ <i className="fa fa-trash" /> Delete
+ </>
+ ),
+ onSelect: confirmDelete,
+ });
+ }
+ return (
+ <ListView
+ className="dataset-list-view"
+ title={'Datasets'}
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -508,71 +510,68 @@ class DashboardList extends React.PureComponent<Props,
State> {
dashboardToEdit,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- <ConfirmStatusChange
- title={t('Please confirm')}
- description={t(
- 'Are you sure you want to delete the selected dashboards?',
- )}
- onConfirm={this.handleBulkDashboardDelete}
- >
- {confirmDelete => {
- const bulkActions = [];
- if (this.canDelete) {
- bulkActions.push({
- key: 'delete',
- name: (
- <>
- <i className="fa fa-trash" /> Delete
- </>
- ),
- onSelect: confirmDelete,
- });
- }
- if (this.canExport) {
- bulkActions.push({
- key: 'export',
- name: (
- <>
- <i className="fa fa-database" /> Export
- </>
- ),
- onSelect: this.handleBulkDashboardExport,
- });
- }
- return (
+ <>
+ <SubMenu name="Dashboards" />
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={t(
+ 'Are you sure you want to delete the selected dashboards?',
+ )}
+ onConfirm={this.handleBulkDashboardDelete}
+ >
+ {confirmDelete => {
+ const bulkActions = [];
+ if (this.canDelete) {
+ bulkActions.push({
+ key: 'delete',
+ name: (
+ <>
+ <i className="fa fa-trash" /> Delete
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/views/chartList/ChartList.tsx
##########
@@ -517,58 +519,55 @@ class ChartList extends React.PureComponent<Props, State>
{
sliceCurrentlyEditing,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- {sliceCurrentlyEditing && (
- <PropertiesModal
- show
- onHide={this.closeChartEditModal}
- onSave={this.handleChartUpdated}
- slice={sliceCurrentlyEditing}
+ <>
+ <SubMenu name="Charts" />
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/views/chartList/ChartList.tsx
##########
@@ -517,58 +519,55 @@ class ChartList extends React.PureComponent<Props, State>
{
sliceCurrentlyEditing,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- {sliceCurrentlyEditing && (
- <PropertiesModal
- show
- onHide={this.closeChartEditModal}
- onSave={this.handleChartUpdated}
- slice={sliceCurrentlyEditing}
+ <>
+ <SubMenu name="Charts" />
+ {sliceCurrentlyEditing && (
+ <PropertiesModal
+ show
+ onHide={this.closeChartEditModal}
+ onSave={this.handleChartUpdated}
+ slice={sliceCurrentlyEditing}
+ />
+ )}
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={t(
+ 'Are you sure you want to delete the selected charts?',
+ )}
+ onConfirm={this.handleBulkChartDelete}
+ >
+ {confirmDelete => {
+ const bulkActions = [];
+ if (this.canDelete) {
+ bulkActions.push({
+ key: 'delete',
+ name: (
+ <>
+ <i className="fa fa-trash" /> Delete
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -508,71 +510,68 @@ class DashboardList extends React.PureComponent<Props,
State> {
dashboardToEdit,
} = this.state;
return (
- <div className="container welcome">
- <Panel>
- <Panel.Body>
- <ConfirmStatusChange
- title={t('Please confirm')}
- description={t(
- 'Are you sure you want to delete the selected dashboards?',
- )}
- onConfirm={this.handleBulkDashboardDelete}
- >
- {confirmDelete => {
- const bulkActions = [];
- if (this.canDelete) {
- bulkActions.push({
- key: 'delete',
- name: (
- <>
- <i className="fa fa-trash" /> Delete
- </>
- ),
- onSelect: confirmDelete,
- });
- }
- if (this.canExport) {
- bulkActions.push({
- key: 'export',
- name: (
- <>
- <i className="fa fa-database" /> Export
- </>
- ),
- onSelect: this.handleBulkDashboardExport,
- });
- }
- return (
+ <>
+ <SubMenu name="Dashboards" />
Review comment:
nit: `t` string here please!
##########
File path: superset-frontend/src/views/datasetList/DatasetList.tsx
##########
@@ -193,36 +195,51 @@ class DatasetList extends React.PureComponent<Props,
State> {
}: any) => kind[0]?.toUpperCase() + kind.slice(1),
Header: t('Type'),
accessor: 'kind',
+ size: 'lg',
},
{
Header: t('Source'),
accessor: 'database_name',
+ size: 'lg',
},
{
Header: t('Schema'),
accessor: 'schema',
+ size: 'lg',
},
{
Cell: ({
row: {
original: { changed_on: changedOn },
},
- }: any) => <span
className="no-wrap">{moment(changedOn).fromNow()}</span>,
+ }: any) => {
+ const momentTime = moment(changedOn);
+ const time = momentTime.format('h:m a');
+ const date = momentTime.format('MMM D, YYYY');
Review comment:
it would be nice to move these format strings into constants (either in
this file or in a more global constant file) assuming that these date and time
formats will be widely used in the future
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
