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/01/16 18:51:23 UTC
[GitHub] [incubator-superset] nytai opened a new pull request #8979:
Tai/bulk actions
nytai opened a new pull request #8979: Tai/bulk actions
URL: https://github.com/apache/incubator-superset/pull/8979
### CATEGORY
Choose one
- [ ] Bug Fix
- [ ] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369752595
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
+ callback: defaultCallback,
+ open: false,
+ };
+
+ public show: ShowCallback = (callback) => (event) => {
Review comment:
Good point. I had modal reuse in mind when I was writing it, but that kind of fell apart once I had to use it in the actions cell renderer. I'll change it, you're right, it's less confusing to pass stuff as props.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369775100
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
Review comment:
oh react
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r370281839
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -21,54 +21,43 @@ import * as React from 'react';
// @ts-ignore
import { Button, Modal } from 'react-bootstrap';
-type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
-
+type Callback = (...args: any[]) => void;
interface Props {
title: string | React.ReactNode;
description: string | React.ReactNode;
- children: (confirm: ShowCallback) => React.ReactNode;
+ onConfirm: Callback;
+ children: (showConfirm: Callback) => React.ReactNode;
}
interface State {
+ callbackArgs: any[];
open: boolean;
- callback: (e: React.MouseEvent) => void;
}
-const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
export default class ConfirmStatusChange extends React.Component<Props, State> {
public state = {
- callback: defaultCallback,
+ callbackArgs: [],
open: false,
};
- public show: ShowCallback = (callback) => (event) => {
- if (typeof event.preventDefault === 'function') {
- event.preventDefault();
-
- event = {
- ...event,
- currentTarget: { ...event.currentTarget },
- };
- }
-
+ public showConfirm = (...callbackArgs: any[]) => {
this.setState({
- callback: () => callback(event),
+ callbackArgs,
Review comment:
Good catch, I was cloning it before. None of my current use cases rely on the dom event, but probably best to handle that to avoid future weirdness.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch merged pull request #8979:
[dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #8979: bulk
actions for dashboard list view
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #8979: bulk actions for dashboard list view
URL: https://github.com/apache/incubator-superset/pull/8979#issuecomment-575855727
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=h1) Report
> Merging [#8979](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2fc5fd4f2921f24e5064bf54561eff87e754409a?src=pr&el=desc) will **increase** coverage by `0.2%`.
> The diff coverage is `68.04%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8979 +/- ##
=========================================
+ Coverage 59.16% 59.37% +0.2%
=========================================
Files 367 369 +2
Lines 11679 11738 +59
Branches 2862 2880 +18
=========================================
+ Hits 6910 6969 +59
Misses 4590 4590
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...assets/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L1RhYmxlQ29sbGVjdGlvbi50c3g=) | `92.59% <ø> (ø)` | :arrow_up: |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <100%> (+0.31%)` | :arrow_up: |
| [...et/assets/src/components/IndeterminateCheckbox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0luZGV0ZXJtaW5hdGVDaGVja2JveC5qc3g=) | `14.28% <14.28%> (ø)` | |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `82.89% <61.11%> (-8.29%)` | :arrow_down: |
| [...t/assets/src/views/dashboardList/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aWV3cy9kYXNoYm9hcmRMaXN0L0Rhc2hib2FyZExpc3QudHN4) | `82.4% <64.51%> (+23.46%)` | :arrow_up: |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `65% <65%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8979?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/8979?src=pr&el=footer). Last update [2fc5fd4...5c960ef](https://codecov.io/gh/apache/incubator-superset/pull/8979?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369895817
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -21,54 +21,43 @@ import * as React from 'react';
// @ts-ignore
import { Button, Modal } from 'react-bootstrap';
-type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
-
+type Callback = (...args: any[]) => void;
interface Props {
title: string | React.ReactNode;
description: string | React.ReactNode;
- children: (confirm: ShowCallback) => React.ReactNode;
+ onConfirm: Callback;
+ children: (showConfirm: Callback) => React.ReactNode;
}
interface State {
+ callbackArgs: any[];
open: boolean;
- callback: (e: React.MouseEvent) => void;
}
-const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
export default class ConfirmStatusChange extends React.Component<Props, State> {
public state = {
- callback: defaultCallback,
+ callbackArgs: [],
open: false,
};
- public show: ShowCallback = (callback) => (event) => {
- if (typeof event.preventDefault === 'function') {
- event.preventDefault();
-
- event = {
- ...event,
- currentTarget: { ...event.currentTarget },
- };
- }
-
+ public showConfirm = (...callbackArgs: any[]) => {
this.setState({
- callback: () => callback(event),
+ callbackArgs,
Review comment:
You'd need to check if the arg is an Event, and call `.persist()` on it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369723084
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
+ callback: defaultCallback,
+ open: false,
+ };
+
+ public show: ShowCallback = (callback) => (event) => {
Review comment:
The callback is held in the closure state until the event triggers at which point it's put into the components state and the modal pops up. Holding the callback in state only happens when the modal is open and we're just waiting for user confirmation before executing the original function. Show can be called multiple times with different callbacks and each callback won't be placed into the state until right before they're about to be executed/cancel (via the modal). Since only one modal can be open at a time I think this makes sense.
What isn't configurable is the message displayed to the user, so that does limit the number of confirmation prompts this can be used with. Passing the title and description to `show` may make sense to enable multiple confirmation messages.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369725283
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
+ callback: defaultCallback,
+ open: false,
+ };
+
+ public show: ShowCallback = (callback) => (event) => {
Review comment:
Oh, I see. I stand corrected. I'm still a little confused by this though. What's the advantage of a stateful setup like this instead of using a separate `ConfirmStatusChange` for each action that needs confirmation?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: bulk actions for dashboard list view
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: bulk actions for dashboard list view
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369170712
##########
File path: superset/assets/spec/javascripts/components/ConfirmStatusChange_spec.jsx
##########
@@ -0,0 +1,56 @@
+/**
+ * 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 { mount } from 'enzyme';
+import { Modal, Button } from 'react-bootstrap';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+
+describe('ConfirmStatusChange', () => {
+ const action = jest.fn();
+ const mockedProps = {
+ title: 'please confirm',
Review comment:
This is just a test, so raw string should be ok.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369725283
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
+ callback: defaultCallback,
+ open: false,
+ };
+
+ public show: ShowCallback = (callback) => (event) => {
Review comment:
Oh, I see. I stand corrected. I'm still a little confused by this though. What's the advantage of a stateful setup like this instead of using a separate `ConfirmStatusChange` for each action that needs confirmation, and passing the callback in as a prop?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8979:
bulk actions for dashboard list view
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8979: bulk actions for dashboard list view
URL: https://github.com/apache/incubator-superset/pull/8979#issuecomment-575855727
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=h1) Report
> Merging [#8979](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/74158694c567ec26706fd3cf8128aed2cb625291?src=pr&el=desc) will **increase** coverage by `0.35%`.
> The diff coverage is `86.59%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8979 +/- ##
==========================================
+ Coverage 59.16% 59.52% +0.35%
==========================================
Files 367 369 +2
Lines 11679 11738 +59
Branches 2862 2880 +18
==========================================
+ Hits 6910 6987 +77
+ Misses 4590 4572 -18
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...assets/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L1RhYmxlQ29sbGVjdGlvbi50c3g=) | `92.59% <ø> (ø)` | :arrow_up: |
| [...et/assets/src/components/IndeterminateCheckbox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0luZGV0ZXJtaW5hdGVDaGVja2JveC5qc3g=) | `100% <100%> (ø)` | |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.1% <100%> (+0.92%)` | :arrow_up: |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <100%> (+0.31%)` | :arrow_up: |
| [...t/assets/src/views/dashboardList/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aWV3cy9kYXNoYm9hcmRMaXN0L0Rhc2hib2FyZExpc3QudHN4) | `82.4% <64.51%> (+23.46%)` | :arrow_up: |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `90% <90%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8979?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/8979?src=pr&el=footer). Last update [7415869...2d58aa6](https://codecov.io/gh/apache/incubator-superset/pull/8979?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369895402
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -21,54 +21,43 @@ import * as React from 'react';
// @ts-ignore
import { Button, Modal } from 'react-bootstrap';
-type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
-
+type Callback = (...args: any[]) => void;
interface Props {
title: string | React.ReactNode;
description: string | React.ReactNode;
- children: (confirm: ShowCallback) => React.ReactNode;
+ onConfirm: Callback;
+ children: (showConfirm: Callback) => React.ReactNode;
}
interface State {
+ callbackArgs: any[];
open: boolean;
- callback: (e: React.MouseEvent) => void;
}
-const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
export default class ConfirmStatusChange extends React.Component<Props, State> {
public state = {
- callback: defaultCallback,
+ callbackArgs: [],
open: false,
};
- public show: ShowCallback = (callback) => (event) => {
- if (typeof event.preventDefault === 'function') {
- event.preventDefault();
-
- event = {
- ...event,
- currentTarget: { ...event.currentTarget },
- };
- }
-
+ public showConfirm = (...callbackArgs: any[]) => {
this.setState({
- callback: () => callback(event),
+ callbackArgs,
Review comment:
If the callback args is supposed to be the DOM event, this will not work as intended. Events can't be referenced asynchronously. https://reactjs.org/docs/events.html#event-pooling
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369326978
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
Review comment:
good point. It was `tslint --fix` that added them and i should have probably done a second pass.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369328509
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
+ callback: defaultCallback,
+ open: false,
+ };
+
+ public show: ShowCallback = (callback) => (event) => {
Review comment:
I think it would make more sense to pass the callback as a prop to the `<ConfirmStatusChange>` component. The callback isn't really "state" as far as I can tell, and it could be misleading to think that you can call `show` multiple times, when that will actually break things.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369754975
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
Review comment:
looks like typescript is complaining about that. React.PureComponet and React.Component both export state as a public property.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369325655
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
Review comment:
This and other methods that are only used in this class should probably not be marked `public`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369754975
##########
File path: superset/assets/src/components/ConfirmStatusChange.tsx
##########
@@ -0,0 +1,87 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import * as React from 'react';
+// @ts-ignore
+import { Button, Modal } from 'react-bootstrap';
+
+type ShowCallback = (callback: (e: any) => any) => (event: any) => any;
+
+interface Props {
+ title: string | React.ReactNode;
+ description: string | React.ReactNode;
+ children: (confirm: ShowCallback) => React.ReactNode;
+}
+
+interface State {
+ open: boolean;
+ callback: (e: React.MouseEvent) => void;
+}
+const defaultCallback = () => { console.error('ConfirmStatusChange invoked with the default callback, please provide a function to be called on confirm'); };
+export default class ConfirmStatusChange extends React.Component<Props, State> {
+
+ public state = {
Review comment:
looks like typescript is complaining about that. React.PureComponet and React.Component both expose state as a public property.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8979:
bulk actions for dashboard list view
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8979: bulk actions for dashboard list view
URL: https://github.com/apache/incubator-superset/pull/8979#issuecomment-575855727
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=h1) Report
> Merging [#8979](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2fc5fd4f2921f24e5064bf54561eff87e754409a?src=pr&el=desc) will **increase** coverage by `0.35%`.
> The diff coverage is `86.59%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8979 +/- ##
==========================================
+ Coverage 59.16% 59.52% +0.35%
==========================================
Files 367 369 +2
Lines 11679 11738 +59
Branches 2862 2880 +18
==========================================
+ Hits 6910 6987 +77
+ Misses 4590 4572 -18
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8979?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...assets/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L1RhYmxlQ29sbGVjdGlvbi50c3g=) | `92.59% <ø> (ø)` | :arrow_up: |
| [...et/assets/src/components/IndeterminateCheckbox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0luZGV0ZXJtaW5hdGVDaGVja2JveC5qc3g=) | `100% <100%> (ø)` | |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.1% <100%> (+0.92%)` | :arrow_up: |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <100%> (+0.31%)` | :arrow_up: |
| [...t/assets/src/views/dashboardList/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aWV3cy9kYXNoYm9hcmRMaXN0L0Rhc2hib2FyZExpc3QudHN4) | `82.4% <64.51%> (+23.46%)` | :arrow_up: |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/8979/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `90% <90%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8979?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/8979?src=pr&el=footer). Last update [2fc5fd4...71f7165](https://codecov.io/gh/apache/incubator-superset/pull/8979?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#8979: bulk actions for dashboard list view
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8979: bulk actions for dashboard list view
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369081029
##########
File path: superset/assets/spec/javascripts/components/ConfirmStatusChange_spec.jsx
##########
@@ -0,0 +1,56 @@
+/**
+ * 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 { mount } from 'enzyme';
+import { Modal, Button } from 'react-bootstrap';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+
+describe('ConfirmStatusChange', () => {
+ const action = jest.fn();
+ const mockedProps = {
+ title: 'please confirm',
Review comment:
`t('please confirm')` ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#8979: [dashboard] new, bulk actions for delete & export
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #8979: [dashboard] new, bulk actions for delete & export
URL: https://github.com/apache/incubator-superset/pull/8979#discussion_r369328817
##########
File path: superset/assets/src/views/dashboardList/DashboardList.tsx
##########
@@ -56,113 +68,120 @@ class DashboardList extends React.PureComponent<Props, State> {
return this.hasPerm('can_delete');
}
+ get canExport() {
+ return this.hasPerm('can_mulexport');
+ }
+
public static propTypes = {
addDangerToast: PropTypes.func.isRequired,
};
public state: State = {
dashboardCount: 0,
dashboards: [],
- deleteCandidate: {},
filterTypes: {},
labelColumns: {},
+ lastFetchDataConfig: null,
loading: false,
permissions: [],
- showDeleteModal: false,
};
- public columns: any = [];
-
public initialSort = [{ id: 'changed_on', desc: true }];
- constructor(props: Props) {
- super(props);
- this.setColumns();
- }
-
- public setColumns = () => {
- this.columns = [
- {
- Cell: ({
- row: {
- original: { url, dashboard_title },
- },
- }: any) => <a href={url}>{dashboard_title}</a>,
- Header: this.state.labelColumns.dashboard_title || '',
- accessor: 'dashboard_title',
- filterable: true,
- sortable: true,
- },
- {
- Cell: ({
- row: {
- original: { changed_by_name, changed_by_url },
- },
- }: any) => <a href={changed_by_url}>{changed_by_name}</a>,
- Header: this.state.labelColumns.changed_by_name || '',
- accessor: 'changed_by_fk',
- sortable: true,
- },
- {
- Cell: ({
- row: {
- original: { published },
- },
- }: any) => (
- <span className='no-wrap'>{published ? <i className='fa fa-check' /> : ''}</span>
- ),
- Header: this.state.labelColumns.published || '',
- accessor: 'published',
- sortable: true,
- },
- {
- Cell: ({
- row: {
- original: { changed_on },
- },
- }: any) => (
- <span className='no-wrap'>{moment(changed_on).fromNow()}</span>
- ),
- Header: this.state.labelColumns.changed_on || '',
- accessor: 'changed_on',
- sortable: true,
- },
- {
- Cell: ({ row: { state, original } }: any) => {
- const handleDelete = () => this.handleDashboardDeleteConfirm(original);
- const handleEdit = () => this.handleDashboardEdit(original);
- if (!this.canEdit && !this.canDelete) {
- return null;
- }
-
- return (
- <span className={`actions ${state && state.hover ? '' : 'invisible'}`}>
- {this.canDelete && (
- <span
- role='button'
- className='action-button'
- onClick={handleDelete}
- >
- <i className='fa fa-trash' />
- </span>
- )}
- {this.canEdit && (
- <span
- role='button'
- className='action-button'
- onClick={handleEdit}
- >
- <i className='fa fa-pencil' />
- </span>
- )}
- </span>
- );
+ public columns = [
+ {
+ Cell: ({
+ row: {
+ original: { url, dashboard_title },
+ },
+ }: any) => <a href={url}>{dashboard_title}</a>,
+ Header: t('Title'),
+ accessor: 'dashboard_title',
+ filterable: true,
+ sortable: true,
+ },
+ {
+ Cell: ({
+ row: {
+ original: { changed_by_name, changed_by_url },
+ },
+ }: any) => <a href={changed_by_url}>{changed_by_name}</a>,
+ Header: t('Changed By Name'),
+ accessor: 'changed_by_fk',
+ sortable: true,
+ },
+ {
+ Cell: ({
+ row: {
+ original: { published },
+ },
+ }: any) => (
+ <span className='no-wrap'>{published ? <i className='fa fa-check' /> : ''}</span>
+ ),
+ Header: t('Published'),
+ accessor: 'published',
+ sortable: true,
+ },
+ {
+ Cell: ({
+ row: {
+ original: { changed_on },
},
- Header: 'Actions',
- id: 'actions',
+ }: any) => (
+ <span className='no-wrap'>{moment(changed_on).fromNow()}</span>
+ ),
+ Header: t('Changed On'),
+ accessor: 'changed_on',
+ sortable: true,
+ },
+ {
+ Cell: ({ row: { state, original } }: any) => {
+ const handleEdit = () => this.handleDashboardEdit(original);
+ const handleExport = () => this.handleBulkDashboardExport([original]);
+ if (!this.canEdit && !this.canDelete && !this.canExport) {
+ return null;
+ }
+
+ return (
+ <ConfirmStatusChange title={t('Please Confirm')} description={`${t('Are you sure you want to delete')} ${original.dashboard_title}?`}>
Review comment:
This `<ConfirmStatusChange>` could wrap just the delete button instead of the entire collection of buttons.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org