bito-code-review[bot] commented on code in PR #34640:
URL: https://github.com/apache/superset/pull/34640#discussion_r3465090856
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -153,121 +208,156 @@ export default function Login() {
};
return (
- <Flex
- justify="center"
- align="center"
- data-test="login-form"
- css={css`
- width: 100%;
- height: calc(100vh - 200px);
- `}
- >
- <StyledCard title={t('Sign in')} padded>
- {authType === AuthType.AuthOID && (
- <Flex justify="center" vertical gap="middle">
- <Form layout="vertical" requiredMark="optional" form={form}>
- {providers.map((provider: OIDProvider) => (
- <Form.Item<LoginForm>>
- <Button
- href={buildProviderLoginUrl(provider.name)}
- block
- iconPosition="start"
- icon={getAuthIconElement(provider.name)}
- >
- {t('Sign in with')} {capitalize(provider.name)}
- </Button>
- </Form.Item>
- ))}
- </Form>
- </Flex>
- )}
- {(authType === AuthType.AuthOauth ||
- authType === AuthType.AuthSAML) && (
- <Flex justify="center" gap={0} vertical>
- <Form layout="vertical" requiredMark="optional" form={form}>
- {providers.map((provider: OAuthProvider) => (
- <Form.Item<LoginForm>>
- <Button
- href={buildProviderLoginUrl(provider.name)}
- block
- iconPosition="start"
- icon={getAuthIconElement(provider.name)}
- >
- {t('Sign in with')} {capitalize(provider.name)}
- </Button>
- </Form.Item>
- ))}
- </Form>
- </Flex>
- )}
-
- {(authType === AuthType.AuthDB || authType === AuthType.AuthLDAP) && (
- <Flex justify="center" vertical gap="middle">
- <Typography.Text type="secondary">
- {t('Enter your login and password below:')}
- </Typography.Text>
- <Form
- layout="vertical"
- requiredMark="optional"
- form={form}
- onFinish={onFinish}
- >
- <Form.Item<LoginForm>
- label={<StyledLabel>{t('Username:')}</StyledLabel>}
- name="username"
- rules={[
- { required: true, message: t('Please enter your username') },
- ]}
- >
- <Input
- autoFocus
- prefix={<Icons.UserOutlined iconSize="l" />}
- data-test="username-input"
+ <>
+ <StyledBackground />
+ <Flex
+ justify="center"
+ align="center"
+ data-test="login-form"
+ css={css`
+ width: 100%;
+ height: calc(100vh - 200px);
+ `}
+ >
+ <StyledCard
+ padded
+ title={
+ <Flex vertical align="start" gap="middle">
+ {theme.loginFormBrandLogoUrl && (
+ <StyledBrandLogo
+ src={theme.loginFormBrandLogoUrl}
+ alt={theme.brandLogoAlt || 'Apache Superset'}
+ style={{ marginTop: theme.sizeUnit * 4 }}
/>
- </Form.Item>
- <Form.Item<LoginForm>
- label={<StyledLabel>{t('Password:')}</StyledLabel>}
- name="password"
- rules={[
- { required: true, message: t('Please enter your password') },
- ]}
+ )}
+ <Typography.Title
+ level={4}
+ style={{
+ marginTop: 0,
+ marginBottom: theme.loginFormBrandLogoUrl
+ ? theme.sizeUnit * 2
+ : 0,
+ marginLeft: 0,
+ marginRight: 0,
+ }}
>
- <Input.Password
- prefix={<Icons.KeyOutlined iconSize="l" />}
- data-test="password-input"
- />
- </Form.Item>
- <Form.Item label={null}>
- <Flex
- css={css`
- width: 100%;
- `}
+ {t('Sign in')}
+ </Typography.Title>
+ </Flex>
+ }
+ >
+ {authType === AuthType.AuthOID && (
+ <Flex justify="center" vertical gap="middle">
+ <Form layout="vertical" requiredMark="optional" form={form}>
+ {providers.map((provider: OIDProvider) => (
+ <Form.Item<LoginForm>>
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing React key prop</b></div>
<div id="fix">
The OID provider map at line 253 is missing a `key` prop. React requires a
unique `key` on list items to enable efficient reconciliation and prevent
unnecessary re-renders. Use `key={provider.name}` to match the pattern used
elsewhere for provider identification.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset-frontend/src/pages/Login/index.tsx (lines 252-263) ---
252: {providers.map((provider: OIDProvider) => (
253: - <Form.Item<LoginForm>>
253:+ <Form.Item<LoginForm> key={provider.name}>
254: <Button
```
</div>
</details>
</div>
<small><i>Code Review Run #32118f</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -153,121 +208,156 @@ export default function Login() {
};
return (
- <Flex
- justify="center"
- align="center"
- data-test="login-form"
- css={css`
- width: 100%;
- height: calc(100vh - 200px);
- `}
- >
- <StyledCard title={t('Sign in')} padded>
- {authType === AuthType.AuthOID && (
- <Flex justify="center" vertical gap="middle">
- <Form layout="vertical" requiredMark="optional" form={form}>
- {providers.map((provider: OIDProvider) => (
- <Form.Item<LoginForm>>
- <Button
- href={buildProviderLoginUrl(provider.name)}
- block
- iconPosition="start"
- icon={getAuthIconElement(provider.name)}
- >
- {t('Sign in with')} {capitalize(provider.name)}
- </Button>
- </Form.Item>
- ))}
- </Form>
- </Flex>
- )}
- {(authType === AuthType.AuthOauth ||
- authType === AuthType.AuthSAML) && (
- <Flex justify="center" gap={0} vertical>
- <Form layout="vertical" requiredMark="optional" form={form}>
- {providers.map((provider: OAuthProvider) => (
- <Form.Item<LoginForm>>
- <Button
- href={buildProviderLoginUrl(provider.name)}
- block
- iconPosition="start"
- icon={getAuthIconElement(provider.name)}
- >
- {t('Sign in with')} {capitalize(provider.name)}
- </Button>
- </Form.Item>
- ))}
- </Form>
- </Flex>
- )}
-
- {(authType === AuthType.AuthDB || authType === AuthType.AuthLDAP) && (
- <Flex justify="center" vertical gap="middle">
- <Typography.Text type="secondary">
- {t('Enter your login and password below:')}
- </Typography.Text>
- <Form
- layout="vertical"
- requiredMark="optional"
- form={form}
- onFinish={onFinish}
- >
- <Form.Item<LoginForm>
- label={<StyledLabel>{t('Username:')}</StyledLabel>}
- name="username"
- rules={[
- { required: true, message: t('Please enter your username') },
- ]}
- >
- <Input
- autoFocus
- prefix={<Icons.UserOutlined iconSize="l" />}
- data-test="username-input"
+ <>
+ <StyledBackground />
+ <Flex
+ justify="center"
+ align="center"
+ data-test="login-form"
+ css={css`
+ width: 100%;
+ height: calc(100vh - 200px);
+ `}
+ >
+ <StyledCard
+ padded
+ title={
+ <Flex vertical align="start" gap="middle">
+ {theme.loginFormBrandLogoUrl && (
+ <StyledBrandLogo
+ src={theme.loginFormBrandLogoUrl}
+ alt={theme.brandLogoAlt || 'Apache Superset'}
+ style={{ marginTop: theme.sizeUnit * 4 }}
/>
- </Form.Item>
- <Form.Item<LoginForm>
- label={<StyledLabel>{t('Password:')}</StyledLabel>}
- name="password"
- rules={[
- { required: true, message: t('Please enter your password') },
- ]}
+ )}
+ <Typography.Title
+ level={4}
+ style={{
+ marginTop: 0,
+ marginBottom: theme.loginFormBrandLogoUrl
+ ? theme.sizeUnit * 2
+ : 0,
+ marginLeft: 0,
+ marginRight: 0,
+ }}
>
- <Input.Password
- prefix={<Icons.KeyOutlined iconSize="l" />}
- data-test="password-input"
- />
- </Form.Item>
- <Form.Item label={null}>
- <Flex
- css={css`
- width: 100%;
- `}
+ {t('Sign in')}
+ </Typography.Title>
+ </Flex>
+ }
+ >
+ {authType === AuthType.AuthOID && (
+ <Flex justify="center" vertical gap="middle">
+ <Form layout="vertical" requiredMark="optional" form={form}>
+ {providers.map((provider: OIDProvider) => (
+ <Form.Item<LoginForm>>
+ <Button
+ href={buildProviderLoginUrl(provider.name)}
+ block
+ iconPosition="start"
+ icon={getAuthIconElement(provider.name)}
+ >
+ {t('Sign in with')} {capitalize(provider.name)}
+ </Button>
+ </Form.Item>
+ ))}
+ </Form>
+ </Flex>
+ )}
+ {(authType === AuthType.AuthOauth ||
+ authType === AuthType.AuthSAML) && (
+ <Flex justify="center" gap={0} vertical>
+ <Form layout="vertical" requiredMark="optional" form={form}>
+ {providers.map((provider: OAuthProvider) => (
+ <Form.Item<LoginForm>>
+ <Button
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing React key prop</b></div>
<div id="fix">
The OAuth/SAML provider map at line 273 is missing a `key` prop. React
requires a unique `key` on list items to enable efficient reconciliation and
prevent unnecessary re-renders. Use `key={provider.name}` to match the pattern
used elsewhere for provider identification.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset-frontend/src/pages/Login/index.tsx (lines 271-282) ---
271: {providers.map((provider: OAuthProvider) => (
272: - <Form.Item<LoginForm>>
272:+ <Form.Item<LoginForm> key={provider.name}>
273: <Button
```
</div>
</details>
</div>
<small><i>Code Review Run #32118f</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
