eschutho commented on a change in pull request #13032:
URL: https://github.com/apache/superset/pull/13032#discussion_r574169580



##########
File path: superset-frontend/src/SqlLab/components/HighlightedSql.jsx
##########
@@ -41,79 +35,76 @@ const propTypes = {
   shrink: PropTypes.bool,
 };
 
-class HighlightedSql extends React.Component {
-  constructor(props) {
-    super(props);
-    this.state = {
-      modalBody: null,
-    };
-  }
+function HighlightedSql({
+  sql,
+  rawSql,
+  maxWidth = 50,
+  maxLines = 5,
+  shrink = false,
+}) {
+  return (
+    <ModalTrigger
+      modalTitle={t('SQL')}
+      modalBody={<Modal rawSql={rawSql} sql={sql} />}
+      triggerNode={
+        <TriggerNode
+          shrink={shrink}
+          sql={sql}
+          maxLines={maxLines}
+          maxWidth={maxWidth}
+        />
+      }
+    />
+  );
+}
+HighlightedSql.propTypes = propTypes;
+
+export default HighlightedSql;
 
-  shrinkSql() {
-    const ssql = this.props.sql || '';
+function TriggerNode({ shrink, sql, maxLines, maxWidth }) {
+  const shrinkSql = () => {
+    const ssql = sql || '';
     let lines = ssql.split('\n');
-    if (lines.length >= this.props.maxLines) {
-      lines = lines.slice(0, this.props.maxLines);
+    if (lines.length >= maxLines) {
+      lines = lines.slice(0, maxLines);
       lines.push('{...}');
     }
     return lines
       .map(line => {
-        if (line.length > this.props.maxWidth) {
-          return `${line.slice(0, this.props.maxWidth)}{...}`;
+        if (line.length > maxWidth) {
+          return `${line.slice(0, maxWidth)}{...}`;
         }
         return line;
       })
       .join('\n');
-  }
+  };
 
-  triggerNode() {
-    const shownSql = this.props.shrink
-      ? this.shrinkSql(this.props.sql)
-      : this.props.sql;
-    return (
-      <SyntaxHighlighter language="sql" style={github}>
-        {shownSql}
-      </SyntaxHighlighter>
-    );
-  }
+  const shownSql = shrink ? shrinkSql() : sql;
+  return (
+    <SyntaxHighlighter language="sql" style={github}>
+      {shownSql}
+    </SyntaxHighlighter>
+  );
+}
 
-  generateModal() {
-    let rawSql;
-    if (this.props.rawSql && this.props.rawSql !== this.props.sql) {
-      rawSql = (
-        <div>
-          <h4>{t('Raw SQL')}</h4>
-          <SyntaxHighlighter language="sql" style={github}>
-            {this.props.rawSql}
-          </SyntaxHighlighter>
-        </div>
-      );
-    }
-    this.setState({
-      modalBody: (
-        <div>
-          <h4>{t('Source SQL')}</h4>
-          <SyntaxHighlighter language="sql" style={github}>
-            {this.props.sql}
-          </SyntaxHighlighter>
+function Modal({ rawSql, sql }) {
+  const generateModal =
+    rawSql && rawSql !== sql ? (
+      <div>
+        <h4>{t('Raw SQL')}</h4>
+        <SyntaxHighlighter language="sql" style={github}>
           {rawSql}
-        </div>
-      ),
-    });
-  }
+        </SyntaxHighlighter>
+      </div>
+    ) : null;
 
-  render() {
-    return (
-      <ModalTrigger
-        modalTitle={t('SQL')}
-        triggerNode={this.triggerNode()}
-        modalBody={this.state.modalBody}
-        beforeOpen={this.generateModal.bind(this)}
-      />
-    );
-  }
+  return (
+    <div>
+      <h4>{t('Source SQL')}</h4>
+      <SyntaxHighlighter language="sql" style={github}>
+        {sql}
+      </SyntaxHighlighter>
+      {generateModal}

Review comment:
       actually I just found one more.. similar to my last comment. You can 
just pull all of line 92-99 here.. no need to declare it above. You can also 
use the logical and (&&) operator instead of ternary if your "or" case is 
`null`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to