korbit-ai[bot] commented on code in PR #34767:
URL: https://github.com/apache/superset/pull/34767#discussion_r2286356462
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -618,13 +570,12 @@ const SqlEditor: FC<Props> = ({
}
};
- const onResizeEnd = ([northPercent, southPercent]: number[]) => {
- setNorthPercent(northPercent);
- setSouthPercent(southPercent);
+ const onResizeEnd = ([nHeight, sHeight]: number[]) => {
+ const northPercent = Math.round((nHeight * 100) / (nHeight + sHeight));
+ const southPercent = 100 - northPercent;
- if (northPaneRef.current?.clientHeight) {
- dispatch(persistEditorHeight(queryEditor, northPercent, southPercent));
- }
+ setNorthPercent(northPercent);
+ dispatch(persistEditorHeight(queryEditor, northPercent, southPercent));
};
Review Comment:
### Potential division by zero in resize handler <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The resize handler could cause a division by zero error if both nHeight and
sHeight are 0, which could happen during initialization or when a user
collapses both panes completely.
###### Why this matters
A division by zero would cause the application to crash or produce NaN
values for the panel percentages, leading to unexpected layout behavior.
###### Suggested change ∙ *Feature Preview*
Add a guard clause to handle the edge case when both heights are 0:
```typescript
const onResizeEnd = ([nHeight, sHeight]: number[]) => {
if (nHeight + sHeight === 0) {
return;
}
const northPercent = Math.round((nHeight * 100) / (nHeight + sHeight));
const southPercent = 100 - northPercent;
setNorthPercent(northPercent);
dispatch(persistEditorHeight(queryEditor, northPercent, southPercent));
};
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/682b76a6-ae13-4a75-a5a8-4c7e76968839/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/682b76a6-ae13-4a75-a5a8-4c7e76968839?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/682b76a6-ae13-4a75-a5a8-4c7e76968839?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/682b76a6-ae13-4a75-a5a8-4c7e76968839?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/682b76a6-ae13-4a75-a5a8-4c7e76968839)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:3de3c870-0f68-4ae0-8a29-cb7e7994abfc -->
[](3de3c870-0f68-4ae0-8a29-cb7e7994abfc)
##########
superset-frontend/src/components/Splitter/index.tsx:
##########
@@ -0,0 +1,21 @@
+/**
+ * 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.
+ */
+
+export { Splitter } from 'antd';
+export type { SplitterProps } from 'antd/es/splitter';
Review Comment:
### Missing Component Abstraction Layer <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Direct re-export of an Ant Design component without abstraction or
customization layer could make future component replacements or modifications
more difficult.
###### Why this matters
If the application needs to change UI libraries or add custom behavior in
the future, changes would need to be made in all places where the component is
used rather than in a single abstraction layer.
###### Suggested change ∙ *Feature Preview*
```typescript
// Create a wrapper component to provide a layer of abstraction
import { Splitter as AntdSplitter } from 'antd';
import type { SplitterProps as AntdSplitterProps } from 'antd/es/splitter';
export type SplitterProps = AntdSplitterProps;
export const Splitter = (props: SplitterProps) => {
return <AntdSplitter {...props} />;
};
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8ac563bb-65d4-4f7b-af32-6e2e293e0492/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8ac563bb-65d4-4f7b-af32-6e2e293e0492?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8ac563bb-65d4-4f7b-af32-6e2e293e0492?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8ac563bb-65d4-4f7b-af32-6e2e293e0492?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8ac563bb-65d4-4f7b-af32-6e2e293e0492)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:37e0b193-9be0-414e-9c44-9b569534c17e -->
[](37e0b193-9be0-414e-9c44-9b569534c17e)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
