-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(ui): update oss login page #17291
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! love the deletion. Couple of comments / question
} | ||
|
||
.cf-dapper-scrollbars--content { | ||
display: flex !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this !important
necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately yes, in clockface there's an !important
present to override styles from a library, so the madness trickles down
width: 70%; | ||
height: auto; | ||
margin-top: 1em; | ||
margin-bottom: 3em; | ||
} | ||
|
||
.signin-page--cubo { | ||
background-image: url('../../assets/images/auth-logo.svg'); | ||
background-size: 100% 100%; | ||
background-position: center center; | ||
background-repeat: no-repeat; | ||
width: 100px; | ||
height: 100px; | ||
display: inline-block; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ninor nit: wanna alphabetize these selectors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that, though makes me think we could benefit from more thorough CSS formatting. I think prettier supports that? Maybe a nice little tooling improvement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexpaxton deffo. I usually just say "alphabetize absent any other organizing scheme. would be rad to have one in place"
<SplashPage.Logo /> | ||
<SplashPage.Header title="InfluxData" /> | ||
<div className="signin-page--cubo" /> | ||
<InfluxDBCloudLogo cloud={false} className="signin-page--logo" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is cloud={false}
we have a CLOUD
global variable that denotes whether we're in a cloud context or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that prop controls whether the word "Cloud" renders in the image. https://influxdata.github.io/clockface/?path=/story/graphics-brand-logos--influxdb-cloud
In this instance I left it false
cuz this page is OSS only
@hoorayimhelping just pushed a commit that sorts the css attributes alphabetically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me, I haven't tested this thougn
Completes half the work described in #17151
The other half is contingent upon this issue: influxdata/clockface#455
FunnelPage
component in the OSS login pageInfluxDBLogo
from Clockface in login panelSplashPage
components and styles