korbit-ai[bot] commented on code in PR #36087:
URL: https://github.com/apache/superset/pull/36087#discussion_r2518330846
##########
superset-frontend/src/features/databases/DatabaseModal/SqlAlchemyForm.tsx:
##########
@@ -97,15 +97,17 @@ const SqlAlchemyTab = ({
</div>
</StyledInputContainer>
{children}
- <Button
- onClick={testConnection}
- loading={testInProgress}
- cta
- buttonStyle="link"
- css={(theme: SupersetTheme) => wideButton(theme)}
- >
- {t('Test connection')}
- </Button>
+ <div css={(theme: SupersetTheme) => marginBottom(theme)}>
+ <Button
+ onClick={testConnection}
+ loading={testInProgress}
+ cta
+ buttonStyle="link"
+ css={(theme: SupersetTheme) => wideButton(theme)}
+ >
+ {t('Test connection')}
+ </Button>
+ </div>
Review Comment:
### Redundant theme function calls <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The theme function is being called twice on every render - once for
marginBottom and once for wideButton - creating unnecessary function calls and
potential style recalculations.
###### Why this matters
This causes redundant theme processing and style computations on each
render, which can impact performance especially when the component re-renders
frequently or when dealing with complex theme objects.
###### Suggested change ∙ *Feature Preview*
Combine the styles into a single CSS function call or memoize the style
calculations:
```tsx
<div css={(theme: SupersetTheme) => [marginBottom(theme), { '& > button':
wideButton(theme) }]}>
<Button
onClick={testConnection}
loading={testInProgress}
cta
buttonStyle="link"
>
{t('Test connection')}
</Button>
</div>
```
Or use useMemo to cache the computed styles.
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/40cd9cae-62f0-49b7-9f77-56485d09edfd/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/40cd9cae-62f0-49b7-9f77-56485d09edfd?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/40cd9cae-62f0-49b7-9f77-56485d09edfd?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/40cd9cae-62f0-49b7-9f77-56485d09edfd?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/40cd9cae-62f0-49b7-9f77-56485d09edfd)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:0fe9facd-a42f-4f62-9379-0543a309d7d9 -->
[](0fe9facd-a42f-4f62-9379-0543a309d7d9)
##########
superset-frontend/src/features/databases/DatabaseModal/styles.ts:
##########
@@ -324,6 +325,7 @@ export const StyledAlignment = styled.div`
export const buttonLinkStyles = (theme: SupersetTheme) => css`
text-transform: initial;
+ padding: 0 ${theme.sizeUnit * 4}px;
padding-right: ${theme.sizeUnit * 2}px;
`;
Review Comment:
### Redundant CSS Padding Declaration <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The padding property is defined twice with conflicting values. The first
padding declaration is immediately overridden by the second padding-right
declaration.
###### Why this matters
This creates confusion in the codebase and makes maintenance more difficult.
The redundant padding declarations could lead to unexpected styling behavior.
###### Suggested change ∙ *Feature Preview*
Combine the padding declarations into a single, clear statement:
```typescript
export const buttonLinkStyles = (theme: SupersetTheme) => css`
text-transform: initial;
padding: 0 ${theme.sizeUnit * 2}px 0 ${theme.sizeUnit * 4}px;
`;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07f7d446-825a-4f91-a6b5-ce13ca975185/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07f7d446-825a-4f91-a6b5-ce13ca975185?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07f7d446-825a-4f91-a6b5-ce13ca975185?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07f7d446-825a-4f91-a6b5-ce13ca975185?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07f7d446-825a-4f91-a6b5-ce13ca975185)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:f72106f3-0f74-4abb-adbe-783e7d8f28b0 -->
[](f72106f3-0f74-4abb-adbe-783e7d8f28b0)
--
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]
