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/12/09 07:33:09 UTC

[GitHub] [incubator-superset] maloun96 opened a new pull request #11971: refactor: Transform URLShortLinkModal to Typescript

maloun96 opened a new pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971


   SUMMARY
   Transform URLShortLinkModal to Typescript


----------------------------------------------------------------
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] junlincc commented on pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#issuecomment-741811090


   @etr2460 🙏


----------------------------------------------------------------
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] maloun96 commented on a change in pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
maloun96 commented on a change in pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#discussion_r540763828



##########
File path: superset-frontend/src/components/URLShortLinkModal.tsx
##########
@@ -17,24 +17,38 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 import CopyToClipboard from './CopyToClipboard';
 import { getShortUrl } from '../utils/common';
 import withToasts from '../messageToasts/enhancers/withToasts';
 import ModalTrigger from './ModalTrigger';
 
-const propTypes = {
-  url: PropTypes.string,
-  emailSubject: PropTypes.string,
-  emailContent: PropTypes.string,
-  addDangerToast: PropTypes.func.isRequired,
-  title: PropTypes.string,
-  triggerNode: PropTypes.node.isRequired,
+type URLShortLinkModalProps = {
+  url?: string;
+  emailSubject?: string;
+  emailContent?: string;
+  title?: string;
+  addDangerToast: (msg: string) => void;
+  triggerNode: JSX.Element;

Review comment:
       what are the differences? I checked and it's not clear




----------------------------------------------------------------
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] etr2460 merged pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
etr2460 merged pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971


   


----------------------------------------------------------------
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] maloun96 commented on a change in pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
maloun96 commented on a change in pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#discussion_r540846288



##########
File path: superset-frontend/src/components/URLShortLinkModal.tsx
##########
@@ -17,24 +17,38 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 import CopyToClipboard from './CopyToClipboard';
 import { getShortUrl } from '../utils/common';
 import withToasts from '../messageToasts/enhancers/withToasts';
 import ModalTrigger from './ModalTrigger';
 
-const propTypes = {
-  url: PropTypes.string,
-  emailSubject: PropTypes.string,
-  emailContent: PropTypes.string,
-  addDangerToast: PropTypes.func.isRequired,
-  title: PropTypes.string,
-  triggerNode: PropTypes.node.isRequired,
+type URLShortLinkModalProps = {
+  url?: string;
+  emailSubject?: string;
+  emailContent?: string;
+  title?: string;
+  addDangerToast: (msg: string) => void;
+  triggerNode: JSX.Element;
 };
 
-class URLShortLinkModal extends React.Component {
-  constructor(props) {
+type URLShortLinkModalState = {
+  shortUrl: string;
+};
+
+class URLShortLinkModal extends React.Component<
+  URLShortLinkModalProps,
+  URLShortLinkModalState
+> {
+  static defaultProps = {
+    url: window.location.href.substring(window.location.origin.length),
+    emailSubject: '',

Review comment:
       changed!




----------------------------------------------------------------
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-io edited a comment on pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#issuecomment-741611029


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=h1) Report
   > Merging [#11971](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=desc) (418d3f3) into [master](https://codecov.io/gh/apache/incubator-superset/commit/7e6f04fce4321cee9d69b14fa003e4c24b5cf720?el=desc) (7e6f04f) will **decrease** coverage by `9.27%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11971/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11971      +/-   ##
   ==========================================
   - Coverage   63.75%   54.48%   -9.28%     
   ==========================================
     Files         941      431     -510     
     Lines       45654    15192   -30462     
     Branches     4389     3885     -504     
   ==========================================
   - Hits        29106     8277   -20829     
   + Misses      16371     6915    -9456     
   + Partials      177        0     -177     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.48% <75.00%> (?)` | |
   | javascript | `?` | |
   | python | `?` | |
   
   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/11971?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <75.00%> (ø)` | |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
   | [...nd/src/views/CRUD/data/query/QueryPreviewModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9xdWVyeS9RdWVyeVByZXZpZXdNb2RhbC50c3g=) | `14.70% <0.00%> (-82.97%)` | :arrow_down: |
   | [...set-frontend/src/views/CRUD/welcome/EmptyState.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9FbXB0eVN0YXRlLnRzeA==) | `5.71% <0.00%> (-82.10%)` | :arrow_down: |
   | ... and [851 more](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?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/11971?src=pr&el=footer). Last update [7e6f04f...418d3f3](https://codecov.io/gh/apache/incubator-superset/pull/11971?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] ktmud commented on a change in pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#discussion_r540774550



##########
File path: superset-frontend/src/components/URLShortLinkModal.tsx
##########
@@ -17,24 +17,38 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 import CopyToClipboard from './CopyToClipboard';
 import { getShortUrl } from '../utils/common';
 import withToasts from '../messageToasts/enhancers/withToasts';
 import ModalTrigger from './ModalTrigger';
 
-const propTypes = {
-  url: PropTypes.string,
-  emailSubject: PropTypes.string,
-  emailContent: PropTypes.string,
-  addDangerToast: PropTypes.func.isRequired,
-  title: PropTypes.string,
-  triggerNode: PropTypes.node.isRequired,
+type URLShortLinkModalProps = {
+  url?: string;
+  emailSubject?: string;
+  emailContent?: string;
+  title?: string;
+  addDangerToast: (msg: string) => void;
+  triggerNode: JSX.Element;

Review comment:
       In this case it’s probably better to use JSX.Element since you’d want the trigger element to always exist and not be a text node.




----------------------------------------------------------------
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-io commented on pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#issuecomment-741611029


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=h1) Report
   > Merging [#11971](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=desc) (418d3f3) into [master](https://codecov.io/gh/apache/incubator-superset/commit/7e6f04fce4321cee9d69b14fa003e4c24b5cf720?el=desc) (7e6f04f) will **decrease** coverage by `9.84%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11971/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11971      +/-   ##
   ==========================================
   - Coverage   63.75%   53.90%   -9.85%     
   ==========================================
     Files         941      431     -510     
     Lines       45654    15192   -30462     
     Branches     4389     3885     -504     
   ==========================================
   - Hits        29106     8190   -20916     
   + Misses      16371     7002    -9369     
   + Partials      177        0     -177     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `53.90% <75.00%> (?)` | |
   | javascript | `?` | |
   | python | `?` | |
   
   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/11971?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <75.00%> (ø)` | |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
   | [...nd/src/views/CRUD/data/query/QueryPreviewModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9xdWVyeS9RdWVyeVByZXZpZXdNb2RhbC50c3g=) | `14.70% <0.00%> (-82.97%)` | :arrow_down: |
   | [...set-frontend/src/views/CRUD/welcome/EmptyState.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9FbXB0eVN0YXRlLnRzeA==) | `5.71% <0.00%> (-82.10%)` | :arrow_down: |
   | ... and [850 more](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?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/11971?src=pr&el=footer). Last update [7e6f04f...418d3f3](https://codecov.io/gh/apache/incubator-superset/pull/11971?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] codecov-io edited a comment on pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#issuecomment-741611029


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=h1) Report
   > Merging [#11971](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=desc) (8a1797d) into [master](https://codecov.io/gh/apache/incubator-superset/commit/7e6f04fce4321cee9d69b14fa003e4c24b5cf720?el=desc) (7e6f04f) will **decrease** coverage by `9.37%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11971/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11971      +/-   ##
   ==========================================
   - Coverage   63.75%   54.37%   -9.38%     
   ==========================================
     Files         941      432     -509     
     Lines       45654    15250   -30404     
     Branches     4389     3891     -498     
   ==========================================
   - Hits        29106     8292   -20814     
   + Misses      16371     6958    -9413     
   + Partials      177        0     -177     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.37% <75.00%> (?)` | |
   | javascript | `?` | |
   | python | `?` | |
   
   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/11971?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <75.00%> (ø)` | |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
   | [...nd/src/views/CRUD/data/query/QueryPreviewModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9xdWVyeS9RdWVyeVByZXZpZXdNb2RhbC50c3g=) | `14.70% <0.00%> (-82.97%)` | :arrow_down: |
   | [...set-frontend/src/views/CRUD/welcome/EmptyState.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9FbXB0eVN0YXRlLnRzeA==) | `5.71% <0.00%> (-82.10%)` | :arrow_down: |
   | ... and [859 more](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?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/11971?src=pr&el=footer). Last update [7e6f04f...8a1797d](https://codecov.io/gh/apache/incubator-superset/pull/11971?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] ktmud commented on a change in pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#discussion_r540772495



##########
File path: superset-frontend/src/components/URLShortLinkModal.tsx
##########
@@ -17,24 +17,38 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 import CopyToClipboard from './CopyToClipboard';
 import { getShortUrl } from '../utils/common';
 import withToasts from '../messageToasts/enhancers/withToasts';
 import ModalTrigger from './ModalTrigger';
 
-const propTypes = {
-  url: PropTypes.string,
-  emailSubject: PropTypes.string,
-  emailContent: PropTypes.string,
-  addDangerToast: PropTypes.func.isRequired,
-  title: PropTypes.string,
-  triggerNode: PropTypes.node.isRequired,
+type URLShortLinkModalProps = {
+  url?: string;
+  emailSubject?: string;
+  emailContent?: string;
+  title?: string;
+  addDangerToast: (msg: string) => void;
+  triggerNode: JSX.Element;

Review comment:
       ReactNode includes JSX.Element (an alias of ReactElement<any, any>), as well as string, null, undefined and node arrays.




----------------------------------------------------------------
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-io edited a comment on pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#issuecomment-741611029


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=h1) Report
   > Merging [#11971](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=desc) (8a1797d) into [master](https://codecov.io/gh/apache/incubator-superset/commit/7e6f04fce4321cee9d69b14fa003e4c24b5cf720?el=desc) (7e6f04f) will **decrease** coverage by `9.87%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11971/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11971?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11971      +/-   ##
   ==========================================
   - Coverage   63.75%   53.88%   -9.88%     
   ==========================================
     Files         941      432     -509     
     Lines       45654    15250   -30404     
     Branches     4389     3891     -498     
   ==========================================
   - Hits        29106     8217   -20889     
   + Misses      16371     7033    -9338     
   + Partials      177        0     -177     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `53.88% <75.00%> (?)` | |
   | javascript | `?` | |
   | python | `?` | |
   
   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/11971?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <75.00%> (ø)` | |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
   | [...nd/src/views/CRUD/data/query/QueryPreviewModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9xdWVyeS9RdWVyeVByZXZpZXdNb2RhbC50c3g=) | `14.70% <0.00%> (-82.97%)` | :arrow_down: |
   | [...set-frontend/src/views/CRUD/welcome/EmptyState.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9FbXB0eVN0YXRlLnRzeA==) | `5.71% <0.00%> (-82.10%)` | :arrow_down: |
   | ... and [858 more](https://codecov.io/gh/apache/incubator-superset/pull/11971/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11971?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/11971?src=pr&el=footer). Last update [7e6f04f...8a1797d](https://codecov.io/gh/apache/incubator-superset/pull/11971?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] etr2460 commented on a change in pull request #11971: refactor: Transform URLShortLinkModal to Typescript

Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #11971:
URL: https://github.com/apache/incubator-superset/pull/11971#discussion_r540334777



##########
File path: superset-frontend/src/components/URLShortLinkModal.tsx
##########
@@ -17,24 +17,38 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 import CopyToClipboard from './CopyToClipboard';
 import { getShortUrl } from '../utils/common';
 import withToasts from '../messageToasts/enhancers/withToasts';
 import ModalTrigger from './ModalTrigger';
 
-const propTypes = {
-  url: PropTypes.string,
-  emailSubject: PropTypes.string,
-  emailContent: PropTypes.string,
-  addDangerToast: PropTypes.func.isRequired,
-  title: PropTypes.string,
-  triggerNode: PropTypes.node.isRequired,
+type URLShortLinkModalProps = {
+  url?: string;
+  emailSubject?: string;
+  emailContent?: string;
+  title?: string;
+  addDangerToast: (msg: string) => void;
+  triggerNode: JSX.Element;
 };
 
-class URLShortLinkModal extends React.Component {
-  constructor(props) {
+type URLShortLinkModalState = {
+  shortUrl: string;
+};
+
+class URLShortLinkModal extends React.Component<
+  URLShortLinkModalProps,
+  URLShortLinkModalState
+> {
+  static defaultProps = {
+    url: window.location.href.substring(window.location.origin.length),
+    emailSubject: '',

Review comment:
       if we default this to empty string, can we make the props type themselves non-undefined? Or maybe get rid of the empty string default val

##########
File path: superset-frontend/src/components/URLShortLinkModal.tsx
##########
@@ -17,24 +17,38 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 import CopyToClipboard from './CopyToClipboard';
 import { getShortUrl } from '../utils/common';
 import withToasts from '../messageToasts/enhancers/withToasts';
 import ModalTrigger from './ModalTrigger';
 
-const propTypes = {
-  url: PropTypes.string,
-  emailSubject: PropTypes.string,
-  emailContent: PropTypes.string,
-  addDangerToast: PropTypes.func.isRequired,
-  title: PropTypes.string,
-  triggerNode: PropTypes.node.isRequired,
+type URLShortLinkModalProps = {
+  url?: string;
+  emailSubject?: string;
+  emailContent?: string;
+  title?: string;
+  addDangerToast: (msg: string) => void;
+  triggerNode: JSX.Element;

Review comment:
       any reason for `JSX.Element` and not `React.ReactNode`?




----------------------------------------------------------------
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