You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/08/05 22:55:53 UTC
[GitHub] [incubator-superset] nytai opened a new pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
nytai opened a new pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
- card viewing mode for dashboards list view
- card viewing mode for charts list view
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
After:
<img width="1678" alt="Screen Shot 2020-08-05 at 3 48 31 PM" src="https://user-images.githubusercontent.com/10255196/89471872-104d6400-d734-11ea-8f21-6e0f6a043bf1.png">
<img width="1679" alt="Screen Shot 2020-08-05 at 3 50 31 PM" src="https://user-images.githubusercontent.com/10255196/89471874-117e9100-d734-11ea-8d07-e839d4887e8a.png">
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
- WIP
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [x] Changes UI (behind `ENABLE_REACT_CRUD_VIEWS` flag)
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469467708
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -87,72 +200,74 @@ export default function TableCollection({
headerGroups,
rows,
loading,
-}: Props) {
+}: TableCollectionProps) {
return (
- <Table {...getTableProps()} className="table table-hover">
- <thead>
- {headerGroups.map(headerGroup => (
- <tr {...headerGroup.getHeaderGroupProps()}>
- {headerGroup.headers.map(column => {
- let sortIcon = <Icon name="sort" />;
- if (column.isSorted && column.isSortedDesc) {
- sortIcon = <Icon name="sort-desc" />;
- } else if (column.isSorted && !column.isSortedDesc) {
- sortIcon = <Icon name="sort-asc" />;
- }
- return column.hidden ? null : (
- <th
- {...column.getHeaderProps(
- column.canSort ? column.getSortByToggleProps() : {},
- )}
- data-test="sort-header"
- className={cx({
- [column.size || '']: column.size,
- })}
- >
- <span>
- {column.render('Header')}
- {column.canSort && sortIcon}
- </span>
- </th>
- );
- })}
- </tr>
- ))}
- </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 || {};
- return (
- <td
- className={cx('table-cell', {
- 'table-cell-loader': loading,
- [cell.column.size || '']: cell.column.size,
+ <TableContainer>
+ <Table {...getTableProps()} className="table table-hover">
+ <thead>
+ {headerGroups.map(headerGroup => (
+ <tr {...headerGroup.getHeaderGroupProps()}>
+ {headerGroup.headers.map(column => {
+ let sortIcon = <Icon name="sort" />;
Review comment:
Not sure if this is easier or harder to parse ¯\\\_(ツ)_/¯ :
`<Icon name={column.isSorted ? (column.isSortedDesc ? "sort-desc" : "sort-desc") : "sort" } />`
Also, you _could_ just set the value of `sortIconName` and then use it inline later in rendering, i.e. `{column.canSort && <Icon name={sortIconName} />`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469452135
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
I guess we could just set transparency stops with two more hex characters, to build hex alpha colors? I.e.
```
background: linear-gradient(
180deg,
${({ theme }) => theme.colors.grayscale.dark2}00 47.83%,
${({ theme }) => theme.colors.grayscale.dark2}00 79.64%,
${({ theme }) => theme.colors.grayscale.dark2}${({ theme }) => theme.opacities.medium} 100%,
``` ,
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
I guess we could just set transparency stops with two more hex characters, to build hex alpha colors? I.e.
```
background: linear-gradient(
180deg,
${({ theme }) => theme.colors.grayscale.dark2}00 47.83%,
${({ theme }) => theme.colors.grayscale.dark2}00 79.64%,
${({ theme }) => theme.colors.grayscale.dark2}${({ theme }) => theme.opacities.medium} 100%,
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r470226081
##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -444,6 +400,146 @@ class DashboardList extends React.PureComponent<Props, State> {
});
};
+ renderActions(original: Dashboard) {
+ const handleDelete = () => this.handleDashboardDelete(original);
+ const handleEdit = () => this.openDashboardEditModal(original);
+ const handleExport = () => this.handleBulkDashboardExport([original]);
+ if (!this.canEdit && !this.canDelete && !this.canExport) {
+ return null;
+ }
+ return (
+ <span className="actions">
+ {this.canDelete && (
+ <ConfirmStatusChange
+ title={t('Please Confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{original.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={handleDelete}
+ >
+ {confirmDelete => (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ <Icon name="trash" />
+ </span>
+ )}
+ </ConfirmStatusChange>
+ )}
+ {this.canExport && (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={handleExport}
+ >
+ <Icon name="share" />
+ </span>
+ )}
+ {this.canEdit && (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={handleEdit}
+ >
+ <Icon name="pencil" />
+ </span>
+ )}
+ </span>
+ );
+ }
+
+ renderCard = (props: Dashboard) => {
+ const menu = (
+ <Menu>
+ {this.canDelete && (
+ <Menu.Item>
+ <ConfirmStatusChange
+ title={t('Please Confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{props.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={() => this.handleDashboardDelete(props)}
+ >
+ {confirmDelete => (
+ <div
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ <ListViewCard.MenuIcon name="trash" /> Delete
+ </div>
+ )}
+ </ConfirmStatusChange>
+ </Menu.Item>
+ )}
+ {this.canExport && (
+ <Menu.Item
+ role="button"
+ tabIndex={0}
+ onClick={() => this.handleBulkDashboardExport([props])}
+ >
+ <ListViewCard.MenuIcon name="share" /> Export
+ </Menu.Item>
+ )}
+ {this.canEdit && (
+ <Menu.Item
+ role="button"
+ tabIndex={0}
+ onClick={() => this.openDashboardEditModal(props)}
+ >
+ <ListViewCard.MenuIcon name="pencil" /> Edit
+ </Menu.Item>
+ )}
+ </Menu>
+ );
+
+ return (
+ <ListViewCard
Review comment:
Ok... might be worth filing a ticket for a rainy day.... all good for now.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r467217123
##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -59,7 +60,7 @@ export default class FaveStar extends React.PureComponent<FaveStarProps> {
? 'favorite-selected'
: 'favorite-unselected'
}
- viewBox="0 0 16 15"
+ viewBox={this.props.viewBox || '0 0 16 16'}
Review comment:
ah good catch, I didn't notice that. All the other icons have a size of 24 and a viewbox of `0 0 24 24`, so we should probably standardize around that and override where necessary.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469452135
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
I guess we could just set transparency stops with two more hex characters, to build hex alpha colors? I.e.
```
background: linear-gradient(
180deg,
${({ theme }) => theme.colors.grayscale.dark2}00 47.83%,
${({ theme }) => theme.colors.grayscale.dark2}00 79.64%,
${({ theme }) => theme.colors.grayscale.dark2}${({ theme }) => theme.opacities.medium} 100%,
100%
``` ,
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469600370
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -257,6 +107,86 @@ const bulkSelectColumnConfig = {
size: 'sm',
};
+const ViewModeContainer = styled.div`
+ padding: 24px 0px 8px 16px;
Review comment:
```suggestion
padding: ${({ theme }) => theme.gridUnit * 6}px 0px ${({ theme }) => theme.gridUnit * 2}px ${({ theme }) => theme.gridUnit * 4}px;
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-670061923
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=h1) Report
> Merging [#10526](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ece91928a9339190163c0bc72b96e51217a90d1e&el=desc) will **decrease** coverage by `4.15%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10526 +/- ##
==========================================
- Coverage 63.60% 59.44% -4.16%
==========================================
Files 763 358 -405
Lines 36120 22918 -13202
Branches 3408 0 -3408
==========================================
- Hits 22973 13624 -9349
+ Misses 13034 9294 -3740
+ Partials 113 0 -113
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `59.44% <ø> (+0.37%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `73.89% <ø> (ø)` | |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-21.06%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.33%)` | :arrow_down: |
| [superset/tasks/slack\_util.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdGFza3Mvc2xhY2tfdXRpbC5weQ==) | `89.47% <0.00%> (-10.53%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| [superset/views/alerts.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYWxlcnRzLnB5) | `66.66% <0.00%> (-4.17%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `85.18% <0.00%> (-2.78%)` | :arrow_down: |
| [superset/models/tags.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3RhZ3MucHk=) | `61.20% <0.00%> (-2.76%)` | :arrow_down: |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `82.50% <0.00%> (-2.76%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.43% <0.00%> (-2.57%)` | :arrow_down: |
| ... and [468 more](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=footer). Last update [ece9192...e58429e](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469452135
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
I guess we could just set transparency stops with two more hex characters, to build hex alpha colors? I.e.
```
background: linear-gradient(
180deg,
${({ theme }) => theme.colors.grayscale.dark2}00 47.83%,
${({ theme }) => theme.colors.grayscale.dark2}00 79.64%,
${({ theme }) => theme.colors.grayscale.dark2}${({ theme }) => theme.opacities.medium} 100%
);
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469643763
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
also, opacities currently include `%`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469375552
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -218,6 +218,7 @@ interface UIFiltersProps {
}
const FilterWrapper = styled.div`
+ display: inline-block;
padding: 24px 16px 8px;
Review comment:
Could use gridUnits for the padding here.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r470222651
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
Ahh yes... we'd need to add hex-based opacities... which seems like they'd be used most if we're proceeding with hex colors.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] riahk commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
riahk commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r467210416
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ justify-content: space-between;
Review comment:
This looks fine when there are 3+ columns, but with 2 columns the cards look like they're pushed to the edges of the container:
<img width="1380" alt="Screen Shot 2020-08-07 at 11 42 57 AM" src="https://user-images.githubusercontent.com/8216382/89677996-3fceae80-d8a3-11ea-8693-e4bb1d2c7b60.png">
Using `justify-content: space-around` would space them more evenly:
<img width="1383" alt="Screen Shot 2020-08-07 at 11 44 40 AM" src="https://user-images.githubusercontent.com/8216382/89678088-6e4c8980-d8a3-11ea-9afe-fdcf59b13fe5.png">
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469640221
##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -444,6 +400,146 @@ class DashboardList extends React.PureComponent<Props, State> {
});
};
+ renderActions(original: Dashboard) {
+ const handleDelete = () => this.handleDashboardDelete(original);
+ const handleEdit = () => this.openDashboardEditModal(original);
+ const handleExport = () => this.handleBulkDashboardExport([original]);
+ if (!this.canEdit && !this.canDelete && !this.canExport) {
+ return null;
+ }
+ return (
+ <span className="actions">
+ {this.canDelete && (
+ <ConfirmStatusChange
+ title={t('Please Confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{original.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={handleDelete}
+ >
+ {confirmDelete => (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ <Icon name="trash" />
+ </span>
+ )}
+ </ConfirmStatusChange>
+ )}
+ {this.canExport && (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={handleExport}
+ >
+ <Icon name="share" />
+ </span>
+ )}
+ {this.canEdit && (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={handleEdit}
+ >
+ <Icon name="pencil" />
+ </span>
+ )}
+ </span>
+ );
+ }
+
+ renderCard = (props: Dashboard) => {
+ const menu = (
+ <Menu>
+ {this.canDelete && (
+ <Menu.Item>
+ <ConfirmStatusChange
+ title={t('Please Confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{props.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={() => this.handleDashboardDelete(props)}
+ >
+ {confirmDelete => (
+ <div
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ <ListViewCard.MenuIcon name="trash" /> Delete
+ </div>
+ )}
+ </ConfirmStatusChange>
+ </Menu.Item>
+ )}
+ {this.canExport && (
+ <Menu.Item
+ role="button"
+ tabIndex={0}
+ onClick={() => this.handleBulkDashboardExport([props])}
+ >
+ <ListViewCard.MenuIcon name="share" /> Export
+ </Menu.Item>
+ )}
+ {this.canEdit && (
+ <Menu.Item
+ role="button"
+ tabIndex={0}
+ onClick={() => this.openDashboardEditModal(props)}
+ >
+ <ListViewCard.MenuIcon name="pencil" /> Edit
+ </Menu.Item>
+ )}
+ </Menu>
+ );
+
+ return (
+ <ListViewCard
Review comment:
I think the chunkiness comes from these large blocks of JSX. There is a lot of markup in these files, however if we move them to their own component we'd have to pass in a lot of the context from this file somehow, things like permissions, handleDashboardDelete, handleDashboardExport, etc.
We could do something like `const createRenderDashboardCard = ({canDelete, canEdit, ..., handleDashboardDelete, ...}) => (props: Dashboard) => (<ListViewCard .....)` but that seems a little less readable, and adds another layer of abstraction, imho.
I don't see any real benefit beyond reducing the number of lines of code in this component class. Similar results could be achieved using code folding features in the text editor.
I do agree that there is a lot of shared logic between the various listviews, around permissions, data fetching, delete endpoints, etc. I'd like to consolidate some of this logic into a single `useListViewResource('dashboard')` hook, but that seems a little beyond the scope of this PR.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch edited a comment on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-669602980
My first impression visually (more of a note for our designers) is that we could have 4 columns in the grid, maybe even 5.
Super stoked to see this!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469607307
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ display: grid;
+ grid-template-columns: repeat(auto-fit, minmax(459px, max-content));
+ grid-gap: 32px;
+ justify-content: center;
+ padding: 8px 16px;
+`;
+
+const CardWrapper = styled.div`
+ display: inline-block;
Review comment:
I think it's good to have a wrapper, this ensures the key is always present. I don't think the implementation of renderCard should have to know that it is responsible for adding the key prop to the resulting JSX. It's something I often always forget. This also provides a wrapper div to implement additional functionality, eg bulk select. I'll remove the unnecessary styling though.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469599905
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -42,173 +44,21 @@ const ListViewStyles = styled.div`
.body {
overflow: scroll;
max-height: 64vh;
-
- table {
- border-collapse: separate;
-
- th {
- background: white;
- position: sticky;
- top: 0;
- &:first-of-type {
- padding-left: ${({ theme }) => theme.gridUnit * 4}px;
- }
- }
- }
- }
-
- .filter-dropdown {
- margin-top: 20px;
- }
-
- .filter-column {
- height: 30px;
- padding: 5px;
- font-size: 16px;
- }
-
- .filter-close {
- height: 30px;
- padding: 5px;
-
- i {
- font-size: 20px;
- }
- }
-
- .table-cell-loader {
- position: relative;
-
- .loading-bar {
- background-color: ${({ theme }) => theme.colors.secondary.light4};
- 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%
- );
- background-size: 200px 48px;
- background-position: -100px 0;
- background-repeat: no-repeat;
- animation: loading-shimmer 1s infinite;
- }
- }
-
- .actions {
- white-space: nowrap;
- font-size: 24px;
- min-width: 100px;
-
- svg,
- i {
- margin-right: 8px;
-
- &:hover {
- path {
- fill: ${({ theme }) => theme.colors.primary.base};
- }
- }
- }
- }
-
- .table-row {
- .actions {
- opacity: 0;
- }
-
- &:hover {
- background-color: ${({ theme }) => theme.colors.secondary.light5};
-
- .actions {
- opacity: 1;
- transition: opacity ease-in ${({ theme }) => theme.transitionTiming}s;
- }
- }
- }
-
- .table-row-selected {
- background-color: ${({ theme }) => theme.colors.secondary.light4};
-
- &:hover {
- background-color: ${({ theme }) => theme.colors.secondary.light4};
- }
- }
-
- .table-cell {
- text-overflow: ellipsis;
- overflow: hidden;
- white-space: nowrap;
- max-width: 300px;
- line-height: 1;
- vertical-align: middle;
- &:first-of-type {
- padding-left: ${({ theme }) => theme.gridUnit * 4}px;
- }
- }
-
- .sort-icon {
- position: absolute;
- }
-
- .form-actions-container {
- position: absolute;
- left: 28px;
- }
-
- .row-count-container {
- float: right;
- padding-right: 24px;
}
}
- @keyframes loading-shimmer {
- 40% {
- background-position: 100% 0;
- }
+ .pagination-container {
+ display: flex;
+ flex-direction: column;
+ justify-content: center;
+ }
- 100% {
- background-position: 100% 0;
- }
+ .row-count-container {
+ margin-top: 8px;
Review comment:
```suggestion
margin-top: ${({ theme }) => theme.gridUnit * 2}px;
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469598723
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ display: grid;
+ grid-template-columns: repeat(auto-fit, minmax(459px, max-content));
+ grid-gap: 32px;
+ justify-content: center;
+ padding: 8px 16px;
+`;
+
+const CardWrapper = styled.div`
+ display: inline-block;
Review comment:
Not sure this is needed, if the parent is a grid?
Actually, I wonder if CardWrapper is needed at all, if the `key` could be passed into the `renderCard` method.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469625623
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
+ rgba(0, 0, 0, 0.219135) 79.64%,
+ rgba(0, 0, 0, 0.5) 100%
+ );
+ }
+`;
+const CardCoverImg = styled.img`
+ display: block;
+ object-fit: cover;
+ width: 459px;
Review comment:
Doesn't seem to like that
<img width="485" alt="Screen Shot 2020-08-12 at 5 32 55 PM" src="https://user-images.githubusercontent.com/10255196/90081320-e6f77f80-dcc1-11ea-9ed1-3982f3921778.png">
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-669602980
My first impression visually (more of a note for our designers) is that we could have 4 columns in the grid, maybe even 5.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469598424
##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -444,6 +400,146 @@ class DashboardList extends React.PureComponent<Props, State> {
});
};
+ renderActions(original: Dashboard) {
+ const handleDelete = () => this.handleDashboardDelete(original);
+ const handleEdit = () => this.openDashboardEditModal(original);
+ const handleExport = () => this.handleBulkDashboardExport([original]);
+ if (!this.canEdit && !this.canDelete && !this.canExport) {
+ return null;
+ }
+ return (
+ <span className="actions">
+ {this.canDelete && (
+ <ConfirmStatusChange
+ title={t('Please Confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{original.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={handleDelete}
+ >
+ {confirmDelete => (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ <Icon name="trash" />
+ </span>
+ )}
+ </ConfirmStatusChange>
+ )}
+ {this.canExport && (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={handleExport}
+ >
+ <Icon name="share" />
+ </span>
+ )}
+ {this.canEdit && (
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={handleEdit}
+ >
+ <Icon name="pencil" />
+ </span>
+ )}
+ </span>
+ );
+ }
+
+ renderCard = (props: Dashboard) => {
+ const menu = (
+ <Menu>
+ {this.canDelete && (
+ <Menu.Item>
+ <ConfirmStatusChange
+ title={t('Please Confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{props.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={() => this.handleDashboardDelete(props)}
+ >
+ {confirmDelete => (
+ <div
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ <ListViewCard.MenuIcon name="trash" /> Delete
+ </div>
+ )}
+ </ConfirmStatusChange>
+ </Menu.Item>
+ )}
+ {this.canExport && (
+ <Menu.Item
+ role="button"
+ tabIndex={0}
+ onClick={() => this.handleBulkDashboardExport([props])}
+ >
+ <ListViewCard.MenuIcon name="share" /> Export
+ </Menu.Item>
+ )}
+ {this.canEdit && (
+ <Menu.Item
+ role="button"
+ tabIndex={0}
+ onClick={() => this.openDashboardEditModal(props)}
+ >
+ <ListViewCard.MenuIcon name="pencil" /> Edit
+ </Menu.Item>
+ )}
+ </Menu>
+ );
+
+ return (
+ <ListViewCard
Review comment:
With this file getting a little chunky, I'm starting to wonder if the Card, the table row, or any other atomic chunks of this file should get broken out into their own components.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-670061923
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=h1) Report
> Merging [#10526](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ece91928a9339190163c0bc72b96e51217a90d1e&el=desc) will **decrease** coverage by `4.17%`.
> The diff coverage is `87.17%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10526 +/- ##
==========================================
- Coverage 63.60% 59.42% -4.18%
==========================================
Files 763 765 +2
Lines 36120 36212 +92
Branches 3408 3439 +31
==========================================
- Hits 22973 21518 -1455
- Misses 13034 14501 +1467
- Partials 113 193 +80
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.98% <87.17%> (+0.11%)` | :arrow_up: |
| #python | `59.09% <ø> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/components/AvatarIcon.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQXZhdGFySWNvbi50c3g=) | `100.00% <ø> (+41.66%)` | :arrow_up: |
| [...erset-frontend/src/components/ListView/Filters.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvRmlsdGVycy50c3g=) | `90.76% <ø> (ø)` | |
| [superset-frontend/src/components/Pagination.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUGFnaW5hdGlvbi50c3g=) | `86.20% <ø> (ø)` | |
| [superset-frontend/src/components/SearchInput.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VhcmNoSW5wdXQudHN4) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/PropertiesModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Qcm9wZXJ0aWVzTW9kYWwudHN4) | `14.92% <ø> (-2.99%)` | :arrow_down: |
| [...et-frontend/src/views/CRUD/dataset/DatasetList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YXNldC9EYXRhc2V0TGlzdC50c3g=) | `73.18% <ø> (ø)` | |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `73.89% <ø> (ø)` | |
| [...rontend/src/views/CRUD/dashboard/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGFzaGJvYXJkL0Rhc2hib2FyZExpc3QudHN4) | `70.06% <70.58%> (-0.02%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `61.74% <76.19%> (-9.35%)` | :arrow_down: |
| [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `94.66% <90.00%> (-3.64%)` | :arrow_down: |
| ... and [163 more](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=footer). Last update [ece9192...1600583](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-669640149
@mistercrunch the screenshots were taken on my retina laptop screen simulating `1680px` width. On my 4k external monitor set to `3008px` width we end up with a 5 column layout.
<img width="3005" alt="Screen Shot 2020-08-05 at 7 05 41 PM" src="https://user-images.githubusercontent.com/10255196/89482409-aee6be80-d74e-11ea-8dd5-ab6e5e1468ed.png">
Initially design specified behavior where the cards would also scale with the window up until a point but we ended up deferring that implementation until we had a better framework for responsiveness in superset.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469607533
##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -21,20 +21,28 @@ import { t } from '@superset-ui/translation';
import PropTypes from 'prop-types';
import React from 'react';
import rison from 'rison';
+import { Label } from 'react-bootstrap';
Review comment:
Did that PR already get merged? I'll rebase and use that.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469608153
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -87,72 +200,74 @@ export default function TableCollection({
headerGroups,
rows,
loading,
-}: Props) {
+}: TableCollectionProps) {
return (
- <Table {...getTableProps()} className="table table-hover">
- <thead>
- {headerGroups.map(headerGroup => (
- <tr {...headerGroup.getHeaderGroupProps()}>
- {headerGroup.headers.map(column => {
- let sortIcon = <Icon name="sort" />;
- if (column.isSorted && column.isSortedDesc) {
- sortIcon = <Icon name="sort-desc" />;
- } else if (column.isSorted && !column.isSortedDesc) {
- sortIcon = <Icon name="sort-asc" />;
- }
- return column.hidden ? null : (
- <th
- {...column.getHeaderProps(
- column.canSort ? column.getSortByToggleProps() : {},
- )}
- data-test="sort-header"
- className={cx({
- [column.size || '']: column.size,
- })}
- >
- <span>
- {column.render('Header')}
- {column.canSort && sortIcon}
- </span>
- </th>
- );
- })}
- </tr>
- ))}
- </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 || {};
- return (
- <td
- className={cx('table-cell', {
- 'table-cell-loader': loading,
- [cell.column.size || '']: cell.column.size,
+ <TableContainer>
+ <Table {...getTableProps()} className="table table-hover">
+ <thead>
+ {headerGroups.map(headerGroup => (
+ <tr {...headerGroup.getHeaderGroupProps()}>
+ {headerGroup.headers.map(column => {
+ let sortIcon = <Icon name="sort" />;
Review comment:
i'm not sure we're allowed nested ternaries per our eslint rules. I've already caused a bug trying to "simplify" this logic once, I'd like to avoid touching this logic again unless necessary.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469600834
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -257,6 +107,86 @@ const bulkSelectColumnConfig = {
size: 'sm',
};
+const ViewModeContainer = styled.div`
+ padding: 24px 0px 8px 16px;
+ display: inline-block;
+ position: relative;
+ top: 8px;
+
+ .toggle-button {
+ display: inline-block;
+ border-radius: 2px;
Review comment:
```suggestion
border-radius: ${({ theme }) => theme.gridUnit / 2}px;
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469449445
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
Hmmmmmmmm.... perhaps outside the scope of this PR, but it'd be nice to find a way to address rgba colors like this on theme variables with a SASS mixin or something, so we can have it do the right thing if we switched to dark mode or something.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469619585
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -31,8 +31,120 @@ interface Props {
loading: boolean;
}
+const TableContainer = styled.div`
+ .table-cell-loader {
+ position: relative;
+
+ .loading-bar {
+ background-color: ${({ theme }) => theme.colors.secondary.light4};
+ 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%
+ );
+ background-size: 200px 48px;
+ background-position: -100px 0;
+ background-repeat: no-repeat;
+ animation: loading-shimmer 1s infinite;
+ }
+ }
+
+ .actions {
+ white-space: nowrap;
+ font-size: 24px;
Review comment:
actually, I don't think this is even necessary anymore. This is left over from the days of the font-awesome icons.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-679291678
Impacts #8976
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r470223486
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
Tracking as a new issue, assigned to me... will tackle it later.
https://github.com/apache/incubator-superset/issues/10598
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469460424
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -31,8 +31,120 @@ interface Props {
loading: boolean;
}
+const TableContainer = styled.div`
+ .table-cell-loader {
+ position: relative;
+
+ .loading-bar {
+ background-color: ${({ theme }) => theme.colors.secondary.light4};
+ 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%
+ );
+ background-size: 200px 48px;
+ background-position: -100px 0;
+ background-repeat: no-repeat;
+ animation: loading-shimmer 1s infinite;
+ }
+ }
+
+ .actions {
+ white-space: nowrap;
+ font-size: 24px;
Review comment:
Hmmmm... the theme has
```
xl: 21,
xxl: 28
```
... I wonder if we should use one of those, and possibly adjust the value if needed. The current theme values are based on the LESS variables file, and might need to be updated to more closely match SIP-34 specs.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-670061923
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=h1) Report
> Merging [#10526](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ece91928a9339190163c0bc72b96e51217a90d1e&el=desc) will **decrease** coverage by `4.51%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10526 +/- ##
==========================================
- Coverage 63.60% 59.09% -4.52%
==========================================
Files 763 354 -409
Lines 36120 22738 -13382
Branches 3408 0 -3408
==========================================
- Hits 22973 13436 -9537
+ Misses 13034 9302 -3732
+ Partials 113 0 -113
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `59.09% <ø> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `73.89% <ø> (ø)` | |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/errors.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXJyb3JzLnB5) | `93.54% <0.00%> (-0.21%)` | :arrow_down: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.30% <0.00%> (-0.01%)` | :arrow_down: |
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/SqlLab/components/ShareSqlLabQuery.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NoYXJlU3FsTGFiUXVlcnkuanN4) | | |
| [...t-frontend/src/dashboard/util/setPeriodicRunner.ts](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3NldFBlcmlvZGljUnVubmVyLnRz) | | |
| [...ntend/src/dashboard/util/backgroundStyleOptions.ts](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2JhY2tncm91bmRTdHlsZU9wdGlvbnMudHM=) | | |
| [.../src/explore/components/FilterDefinitionOption.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9GaWx0ZXJEZWZpbml0aW9uT3B0aW9uLmpzeA==) | | |
| [...end/src/components/Select/WindowedSelect/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1dpbmRvd2VkU2VsZWN0L2luZGV4LnRzeA==) | | |
| ... and [393 more](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=footer). Last update [ece9192...1600583](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469601220
##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -21,20 +21,28 @@ import { t } from '@superset-ui/translation';
import PropTypes from 'prop-types';
import React from 'react';
import rison from 'rison';
+import { Label } from 'react-bootstrap';
import {
createFetchRelated,
createErrorHandler,
createFaveStarHandlers,
} from 'src/views/CRUD/utils';
import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
import SubMenu from 'src/components/Menu/SubMenu';
-import ListView, { ListViewProps } from 'src/components/ListView/ListView';
+import AvatarIcon from 'src/components/AvatarIcon';
+import ListView, {
+ ListViewProps,
+ FetchDataConfig,
+ Filters,
+} from 'src/components/ListView';
import ExpandableList from 'src/components/ExpandableList';
-import { FetchDataConfig, Filters } from 'src/components/ListView/types';
+import Owner from 'src/types/Owner';
import withToasts from 'src/messageToasts/enhancers/withToasts';
import Icon from 'src/components/Icon';
import FaveStar from 'src/components/FaveStar';
import PropertiesModal from 'src/dashboard/components/PropertiesModal';
+import ListViewCard from 'src/components/ListViewCard';
+import { Dropdown, Menu } from 'src/common/components';
Review comment:
🎉
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r467272837
##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -59,7 +60,7 @@ export default class FaveStar extends React.PureComponent<FaveStarProps> {
? 'favorite-selected'
: 'favorite-unselected'
}
- viewBox="0 0 16 15"
+ viewBox={this.props.viewBox || '0 0 16 16'}
Review comment:
Good catch, I'm not sure what I was thinking. Looks like it's not actually applying this for my use case since I didn't add a tooltip prop. Reverting this back...
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469454791
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
+ rgba(0, 0, 0, 0.219135) 79.64%,
+ rgba(0, 0, 0, 0.5) 100%
+ );
+ }
+`;
+const CardCoverImg = styled.img`
+ display: block;
+ object-fit: cover;
+ width: 459px;
Review comment:
Could just be 100% width, and auto height if the card has its own width set, I think?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r467276461
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ justify-content: space-between;
Review comment:
I ended up ditching the flexbox implementation in favor of CSS grid. I really didn't like how this was sometimes happening on the last row
<img width="1844" alt="Screen Shot 2020-08-07 at 2 24 20 PM" src="https://user-images.githubusercontent.com/10255196/89689683-c4c4c280-d8b9-11ea-861f-ee3b6aaaaeb0.png">
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-670321132
The rails look very wide, how do other tools deal with this. [off to pinterest.com ...] Looks like the rails between cards is constant, but the leftmost and rightmost rails are dynamic and the cards that fit in are centered.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai merged pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai merged pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469600748
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -257,6 +107,86 @@ const bulkSelectColumnConfig = {
size: 'sm',
};
+const ViewModeContainer = styled.div`
+ padding: 24px 0px 8px 16px;
+ display: inline-block;
+ position: relative;
+ top: 8px;
+
+ .toggle-button {
+ display: inline-block;
+ border-radius: 2px;
+ padding: 4px 4px 0 4px;
Review comment:
```suggestion
padding: ${({ theme }) => theme.gridUnit}px;
padding-bottom: 0;
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469459429
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -31,8 +31,120 @@ interface Props {
loading: boolean;
}
+const TableContainer = styled.div`
+ .table-cell-loader {
+ position: relative;
+
+ .loading-bar {
+ background-color: ${({ theme }) => theme.colors.secondary.light4};
+ 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%
+ );
+ background-size: 200px 48px;
+ background-position: -100px 0;
+ background-repeat: no-repeat;
+ animation: loading-shimmer 1s infinite;
+ }
+ }
+
+ .actions {
+ white-space: nowrap;
+ font-size: 24px;
+ min-width: 100px;
+
+ svg,
+ i {
+ margin-right: 8px;
+
+ &:hover {
+ path {
+ fill: ${({ theme }) => theme.colors.primary.base};
+ }
+ }
+ }
+ }
+
+ .table-row {
+ .actions {
+ opacity: 0;
+ }
+
+ &:hover {
+ background-color: ${({ theme }) => theme.colors.secondary.light5};
+
+ .actions {
+ opacity: 1;
+ transition: opacity ease-in ${({ theme }) => theme.transitionTiming}s;
+ }
+ }
+ }
+
+ .table-row-selected {
+ background-color: ${({ theme }) => theme.colors.secondary.light4};
+
+ &:hover {
+ background-color: ${({ theme }) => theme.colors.secondary.light4};
+ }
+ }
+
+ .table-cell {
+ text-overflow: ellipsis;
+ overflow: hidden;
+ white-space: nowrap;
+ max-width: 300px;
+ line-height: 1;
+ vertical-align: middle;
+ &:first-of-type {
+ padding-left: ${({ theme }) => theme.gridUnit * 4}px;
+ }
+ }
+
+ .sort-icon {
+ position: absolute;
+ }
+
+ @keyframes loading-shimmer {
+ 40% {
+ background-position: 100% 0;
+ }
+
+ 100% {
+ background-position: 100% 0;
+ }
+ }
+`;
+
const Table = styled.table`
+ border-collapse: separate;
+
th {
+ background: white;
Review comment:
theme.colors.grayscale.light5
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r470224928
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ display: grid;
+ grid-template-columns: repeat(auto-fit, minmax(459px, max-content));
Review comment:
Created [this issue](https://github.com/apache/incubator-superset/issues/10599) for these 459px widths (there are a couple in here) and self assigned it, for following up... whenever.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469611947
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ display: grid;
+ grid-template-columns: repeat(auto-fit, minmax(459px, max-content));
Review comment:
happy to move the spacing to use gridUnit. I pulled the style for `grid-template-column` off a stack overflow post, so I have no idea if there is a better way to do this... I just found something that's good enough
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-670061923
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=h1) Report
> Merging [#10526](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ece91928a9339190163c0bc72b96e51217a90d1e&el=desc) will **decrease** coverage by `4.67%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10526 +/- ##
==========================================
- Coverage 63.60% 58.92% -4.68%
==========================================
Files 763 354 -409
Lines 36120 22738 -13382
Branches 3408 0 -3408
==========================================
- Hits 22973 13398 -9575
+ Misses 13034 9340 -3694
+ Partials 113 0 -113
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `58.92% <ø> (-0.16%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `73.89% <ø> (ø)` | |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-21.06%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.72% <0.00%> (-12.77%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `85.18% <0.00%> (-2.78%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.27% <0.00%> (-1.95%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `86.88% <0.00%> (-1.64%)` | :arrow_down: |
| [superset/jinja\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvamluamFfY29udGV4dC5weQ==) | `80.00% <0.00%> (-0.96%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.32% <0.00%> (-0.29%)` | :arrow_down: |
| ... and [403 more](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=footer). Last update [ece9192...1600583](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] riahk commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
riahk commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r466680815
##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -59,7 +60,7 @@ export default class FaveStar extends React.PureComponent<FaveStarProps> {
? 'favorite-selected'
: 'favorite-unselected'
}
- viewBox="0 0 16 15"
+ viewBox={this.props.viewBox || '0 0 16 16'}
Review comment:
Why the change here? The previous `viewBox` matches the prop on the svg itself, unless we want to update that one as well.
##########
File path: superset-frontend/src/components/Icon/index.tsx
##########
@@ -64,7 +66,9 @@ type IconName =
| 'sort-desc'
| 'trash'
| 'share'
- | 'warning';
+ | 'warning'
+ | 'list-view'
+ | 'card-view';
Review comment:
v small nit but let's keep this list (and the one below) alphabetical
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r467217123
##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -59,7 +60,7 @@ export default class FaveStar extends React.PureComponent<FaveStarProps> {
? 'favorite-selected'
: 'favorite-unselected'
}
- viewBox="0 0 16 15"
+ viewBox={this.props.viewBox || '0 0 16 16'}
Review comment:
ah good catch, I didn't notice that. All the other icons have a size of 24 and a viewbox of `0 0 24 24`, so we should probably standardize around that and override where necessary.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469463080
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -87,72 +200,74 @@ export default function TableCollection({
headerGroups,
rows,
loading,
-}: Props) {
+}: TableCollectionProps) {
return (
- <Table {...getTableProps()} className="table table-hover">
- <thead>
- {headerGroups.map(headerGroup => (
- <tr {...headerGroup.getHeaderGroupProps()}>
- {headerGroup.headers.map(column => {
- let sortIcon = <Icon name="sort" />;
- if (column.isSorted && column.isSortedDesc) {
- sortIcon = <Icon name="sort-desc" />;
- } else if (column.isSorted && !column.isSortedDesc) {
- sortIcon = <Icon name="sort-asc" />;
- }
- return column.hidden ? null : (
- <th
- {...column.getHeaderProps(
- column.canSort ? column.getSortByToggleProps() : {},
- )}
- data-test="sort-header"
- className={cx({
- [column.size || '']: column.size,
- })}
- >
- <span>
- {column.render('Header')}
- {column.canSort && sortIcon}
- </span>
- </th>
- );
- })}
- </tr>
- ))}
- </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 || {};
- return (
- <td
- className={cx('table-cell', {
- 'table-cell-loader': loading,
- [cell.column.size || '']: cell.column.size,
+ <TableContainer>
Review comment:
If `Table` is a styled `table` is there any need for this `TableContainer` to exist? Seems like those styles could just be part of the `Table` styles.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469594816
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ display: grid;
+ grid-template-columns: repeat(auto-fit, minmax(459px, max-content));
Review comment:
again wondering about the origin of the 459 value, wondering if it should be something like `calc(some% - 1px)`
And the grid-gap and padding could be `gridUnit` based.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469447321
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
Review comment:
In general, it'd be good to use gridUnits, but this one jumped out at me... why 459 specifically? There's also a 264px height below on the Cover styles, which seems like it should be propped open by its contents.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469600934
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -257,6 +107,86 @@ const bulkSelectColumnConfig = {
size: 'sm',
};
+const ViewModeContainer = styled.div`
+ padding: 24px 0px 8px 16px;
+ display: inline-block;
+ position: relative;
+ top: 8px;
+
+ .toggle-button {
+ display: inline-block;
+ border-radius: 2px;
+ padding: 4px 4px 0 4px;
+
+ &:first-of-type {
+ margin-right: 8px;
Review comment:
```suggestion
margin-right: ${({ theme }) => theme.gridUnit * 2}px;
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469447775
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
Review comment:
theme.transitionTiming is 0.3 if you wanna base this on that var.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469607307
##########
File path: superset-frontend/src/components/ListView/CardCollection.tsx
##########
@@ -0,0 +1,61 @@
+/**
+ * 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 { TableInstance } from 'react-table';
+import styled from '@superset-ui/style';
+
+interface Props {
+ renderCard?: (row: any) => React.ReactNode;
+ prepareRow: TableInstance['prepareRow'];
+ rows: TableInstance['rows'];
+ loading: boolean;
+}
+
+const CardContainer = styled.div`
+ display: grid;
+ grid-template-columns: repeat(auto-fit, minmax(459px, max-content));
+ grid-gap: 32px;
+ justify-content: center;
+ padding: 8px 16px;
+`;
+
+const CardWrapper = styled.div`
+ display: inline-block;
Review comment:
I think it's good to have a wrapper, this ensures the key is always present. I don't think the implementation of renderCard should have to know that it is responsible for adding the key prop to the resulting JSX. It's something I often forget. This also provides a wrapper div to implement additional functionality, eg bulk select. I'll remove the unnecessary styling though.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469624828
##########
File path: superset-frontend/src/components/ListViewCard/index.tsx
##########
@@ -0,0 +1,196 @@
+/**
+ * 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 styled from '@superset-ui/style';
+import Icon from 'src/components/Icon';
+import { Card } from 'src/common/components';
+
+const MenuIcon = styled(Icon)`
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 1px;
+`;
+
+const ActionsWrapper = styled.div`
+ width: 64px;
+ display: flex;
+ justify-content: space-between;
+`;
+
+const StyledCard = styled(Card)`
+ width: 459px;
+
+ .ant-card-body {
+ padding: 16px 8px;
+ }
+ .ant-card-meta-detail > div:not(:last-child) {
+ margin-bottom: 0;
+ }
+`;
+
+const Cover = styled.div`
+ height: 264px;
+ overflow: hidden;
+
+ .cover-footer {
+ transform: translateY(36px);
+ transition: 0.2s ease-out;
+ }
+
+ &:hover {
+ .cover-footer {
+ transform: translateY(0);
+ }
+ }
+`;
+
+const GradientContainer = styled.div`
+ position: relative;
+ display: inline-block;
+
+ &:after {
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100%;
+ height: 100%;
+ display: inline-block;
+ background: linear-gradient(
+ 180deg,
+ rgba(0, 0, 0, 0) 47.83%,
Review comment:
I tried this, eslint or prettier is refusing to keep `theme.colors.grayscale.dark2}${({ theme }) => theme.opacities.medium} 100%` on the same line
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#issuecomment-670061923
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=h1) Report
> Merging [#10526](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ece91928a9339190163c0bc72b96e51217a90d1e&el=desc) will **decrease** coverage by `4.17%`.
> The diff coverage is `87.17%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10526 +/- ##
==========================================
- Coverage 63.60% 59.42% -4.18%
==========================================
Files 763 765 +2
Lines 36120 36212 +92
Branches 3408 3439 +31
==========================================
- Hits 22973 21519 -1454
- Misses 13034 14500 +1466
- Partials 113 193 +80
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.98% <87.17%> (+0.11%)` | :arrow_up: |
| #python | `59.09% <ø> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/components/AvatarIcon.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQXZhdGFySWNvbi50c3g=) | `100.00% <ø> (+41.66%)` | :arrow_up: |
| [...erset-frontend/src/components/ListView/Filters.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvRmlsdGVycy50c3g=) | `90.76% <ø> (ø)` | |
| [superset-frontend/src/components/Pagination.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUGFnaW5hdGlvbi50c3g=) | `86.20% <ø> (ø)` | |
| [superset-frontend/src/components/SearchInput.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VhcmNoSW5wdXQudHN4) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/PropertiesModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Qcm9wZXJ0aWVzTW9kYWwudHN4) | `14.92% <ø> (-2.99%)` | :arrow_down: |
| [...et-frontend/src/views/CRUD/dataset/DatasetList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YXNldC9EYXRhc2V0TGlzdC50c3g=) | `73.18% <ø> (ø)` | |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `73.89% <ø> (ø)` | |
| [...rontend/src/views/CRUD/dashboard/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGFzaGJvYXJkL0Rhc2hib2FyZExpc3QudHN4) | `70.06% <70.58%> (-0.02%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `61.74% <76.19%> (-9.35%)` | :arrow_down: |
| [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `94.66% <90.00%> (-3.64%)` | :arrow_down: |
| ... and [162 more](https://codecov.io/gh/apache/incubator-superset/pull/10526/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=footer). Last update [ece9192...1600583](https://codecov.io/gh/apache/incubator-superset/pull/10526?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469602007
##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -21,20 +21,28 @@ import { t } from '@superset-ui/translation';
import PropTypes from 'prop-types';
import React from 'react';
import rison from 'rison';
+import { Label } from 'react-bootstrap';
Review comment:
Could this be the Label from `src/components/Label`?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10526: feat: SIP-34 card/grid views for dashboards and charts
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #10526:
URL: https://github.com/apache/incubator-superset/pull/10526#discussion_r469468475
##########
File path: superset-frontend/src/components/ListView/TableCollection.tsx
##########
@@ -87,72 +200,74 @@ export default function TableCollection({
headerGroups,
rows,
loading,
-}: Props) {
+}: TableCollectionProps) {
return (
- <Table {...getTableProps()} className="table table-hover">
- <thead>
- {headerGroups.map(headerGroup => (
- <tr {...headerGroup.getHeaderGroupProps()}>
- {headerGroup.headers.map(column => {
- let sortIcon = <Icon name="sort" />;
- if (column.isSorted && column.isSortedDesc) {
- sortIcon = <Icon name="sort-desc" />;
- } else if (column.isSorted && !column.isSortedDesc) {
- sortIcon = <Icon name="sort-asc" />;
- }
- return column.hidden ? null : (
- <th
- {...column.getHeaderProps(
- column.canSort ? column.getSortByToggleProps() : {},
- )}
- data-test="sort-header"
- className={cx({
- [column.size || '']: column.size,
- })}
- >
- <span>
- {column.render('Header')}
- {column.canSort && sortIcon}
- </span>
- </th>
- );
- })}
- </tr>
- ))}
- </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 || {};
- return (
- <td
- className={cx('table-cell', {
- 'table-cell-loader': loading,
- [cell.column.size || '']: cell.column.size,
+ <TableContainer>
+ <Table {...getTableProps()} className="table table-hover">
+ <thead>
+ {headerGroups.map(headerGroup => (
+ <tr {...headerGroup.getHeaderGroupProps()}>
+ {headerGroup.headers.map(column => {
+ let sortIcon = <Icon name="sort" />;
Review comment:
These are just indentation changes, so sorry if I'm off target ;)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org