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 2022/01/18 21:59:47 UTC
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #17807: refactor: sqleditorleftbar to functional
lyndsiWilliams commented on a change in pull request #17807:
URL: https://github.com/apache/superset/pull/17807#discussion_r787149772
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -51,98 +51,35 @@ const StyledScrollbarContent = styled.div`
height: ${props => props.contentHeight}px;
`;
-export default class SqlEditorLeftBar extends React.PureComponent {
- constructor(props) {
- super(props);
- this.resetState = this.resetState.bind(this);
- this.onSchemaChange = this.onSchemaChange.bind(this);
- this.onSchemasLoad = this.onSchemasLoad.bind(this);
- this.onTablesLoad = this.onTablesLoad.bind(this);
- this.onDbChange = this.onDbChange.bind(this);
- this.getDbList = this.getDbList.bind(this);
- this.onTableChange = this.onTableChange.bind(this);
- this.onToggleTable = this.onToggleTable.bind(this);
- }
-
- onSchemaChange(schema) {
- this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
- }
-
- onSchemasLoad(schemas) {
- this.props.actions.queryEditorSetSchemaOptions(
- this.props.queryEditor,
- schemas,
- );
- }
-
- onTablesLoad(tables) {
- this.props.actions.queryEditorSetTableOptions(
- this.props.queryEditor,
- tables,
- );
- }
-
- onDbChange(db) {
- this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- db.id,
- );
- }
-
- onTableChange(tableName, schemaName) {
+export default function SqlEditorLeftBar({
Review comment:
Please change this to an arrow function
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +98,73 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = height - 130; // 130 is the height of the selects above
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={database}
+ dbId={queryEditor.dbId}
+ getDbList={actions.setDatabases}
+ handleError={actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={actions.queryEditorSetSchema}
+ onSchemasLoad={actions.queryEditorSetSchemaOptions}
+ onTableChange={onTableChange}
+ onTablesLoad={actions.queryEditorSetTableOptions}
+ schema={queryEditor.schema}
+ sqlLabMode
+ />
+ <div className="divider" />
+ <StyledScrollbarContainer>
+ <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
+ <Collapse
+ activeKey={tb
+ .filter(({ expanded }) => expanded)
+ .map(({ id }) => id)}
+ css={theme => css`
Review comment:
These style definitions are a little long, can you move them to an external variable? As per [the style guide](https://github.com/apache/superset/issues/15264).
##########
File path: superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.jsx
##########
@@ -161,79 +98,73 @@ export default class SqlEditorLeftBar extends React.PureComponent {
</IconTooltip>
);
- render() {
- const shouldShowReset = window.location.search === '?reset=1';
- const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above
- const qe = this.props.queryEditor;
- return (
- <div className="SqlEditorLeftBar">
- <TableSelector
- database={this.props.database}
- dbId={qe.dbId}
- getDbList={this.getDbList}
- handleError={this.props.actions.addDangerToast}
- onDbChange={this.onDbChange}
- onSchemaChange={this.onSchemaChange}
- onSchemasLoad={this.onSchemasLoad}
- onTableChange={this.onTableChange}
- onTablesLoad={this.onTablesLoad}
- schema={qe.schema}
- sqlLabMode
- />
- <div className="divider" />
- <StyledScrollbarContainer>
- <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
- <Collapse
- activeKey={this.props.tables
- .filter(({ expanded }) => expanded)
- .map(({ id }) => id)}
- css={theme => css`
- .ant-collapse-item {
- margin-bottom: ${theme.gridUnit * 3}px;
- }
- .ant-collapse-header {
- padding: 0px !important;
- display: flex;
- align-items: center;
- }
- .ant-collapse-content-box {
- padding: 0px ${theme.gridUnit * 4}px 0px 0px !important;
- }
- .ant-collapse-arrow {
- top: ${theme.gridUnit * 2}px !important;
- color: ${theme.colors.primary.dark1} !important;
- &: hover {
- color: ${theme.colors.primary.dark2} !important;
- }
+ const shouldShowReset = window.location.search === '?reset=1';
+ const tableMetaDataHeight = height - 130; // 130 is the height of the selects above
+
+ return (
+ <div className="SqlEditorLeftBar">
+ <TableSelector
+ database={database}
+ dbId={queryEditor.dbId}
+ getDbList={actions.setDatabases}
+ handleError={actions.addDangerToast}
+ onDbChange={onDbChange}
+ onSchemaChange={actions.queryEditorSetSchema}
+ onSchemasLoad={actions.queryEditorSetSchemaOptions}
+ onTableChange={onTableChange}
+ onTablesLoad={actions.queryEditorSetTableOptions}
+ schema={queryEditor.schema}
+ sqlLabMode
+ />
+ <div className="divider" />
+ <StyledScrollbarContainer>
+ <StyledScrollbarContent contentHeight={tableMetaDataHeight}>
Review comment:
```suggestion
<div
css={css`
height: ${props => props.contentHeight}px;
`}
contentHeight={tableMetaDataHeight}
>
```
Since this style definition is only one line, it doesn't need to be in an external variable.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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