[GitHub] [incubator-superset] etr2460 commented on a change in pull request #11971: refactor: Transform URLShortLinkModal to Typescript

[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