msyavuz commented on code in PR #34640:
URL: https://github.com/apache/superset/pull/34640#discussion_r2267648631
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -74,7 +74,62 @@ const StyledLabel = styled(Typography.Text)`
`}
`;
+const StyledBackground = styled.div`
+ ${({ theme }) => {
+ const bgImageUrl = theme.loginPageBackgroundImageUrl;
+ const overlayColor =
+ theme.loginPageBackgroundOverlayColor || 'rgba(0, 0, 0, 0.5)';
Review Comment:
How does this default work with dark mode now?
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -74,7 +74,62 @@ const StyledLabel = styled(Typography.Text)`
`}
`;
+const StyledBackground = styled.div`
+ ${({ theme }) => {
+ const bgImageUrl = theme.loginPageBackgroundImageUrl;
+ const overlayColor =
+ theme.loginPageBackgroundOverlayColor || 'rgba(0, 0, 0, 0.5)';
+ const blurAmount = theme.loginPageBackgroundBlur || '0px';
+ return bgImageUrl
+ ? css`
+ position: fixed;
+ top: 0;
+ left: 0;
+ width: 100vw;
+ height: 100vh;
+ z-index: 0;
+ pointer-events: none;
+
+ &::before {
+ content: '';
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 100%;
+ height: 100%;
+ background-image: url(${bgImageUrl});
+ background-size: cover;
+ background-position: center;
+ filter: blur(${blurAmount});
+ z-index: -1;
+ }
+
+ &::after {
+ content: '';
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 100%;
+ height: 100%;
+ background-color: ${overlayColor};
+ z-index: 1;
+ }
Review Comment:
Are these psuedo element styles necessary? I think we can simplify this a
bit further. Something like this with more tweaks:
```suggestion
background: ${ bgImageUrl ?
url(bgImageUrl) : overlayColor }
```
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -74,7 +74,62 @@ const StyledLabel = styled(Typography.Text)`
`}
`;
+const StyledBackground = styled.div`
+ ${({ theme }) => {
+ const bgImageUrl = theme.loginPageBackgroundImageUrl;
+ const overlayColor =
+ theme.loginPageBackgroundOverlayColor || 'rgba(0, 0, 0, 0.5)';
+ const blurAmount = theme.loginPageBackgroundBlur || '0px';
+ return bgImageUrl
+ ? css`
+ position: fixed;
+ top: 0;
+ left: 0;
+ width: 100vw;
+ height: 100vh;
+ z-index: 0;
+ pointer-events: none;
+
+ &::before {
+ content: '';
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 100%;
+ height: 100%;
+ background-image: url(${bgImageUrl});
+ background-size: cover;
+ background-position: center;
+ filter: blur(${blurAmount});
+ z-index: -1;
+ }
+
+ &::after {
+ content: '';
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 100%;
+ height: 100%;
+ background-color: ${overlayColor};
+ z-index: 1;
+ }
+ `
+ : '';
+ }}
+`;
+
+const StyledBrandLogo = styled.img`
+ display: block;
+ max-width: 160px;
+ max-height: 64px;
+ margin: 0;
+ object-fit: contain;
+`;
Review Comment:
We do a very similar thing with Main navbar brand logo. Maybe we can extract
that out to a component and use that here as well? Also enable us to unify
existing brand logo tokens and this new one. (add a config option for
displaying brand logo in login for example)
--
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]
