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(nms): Add new style to the Login page #13403
Conversation
Thanks for opening a PR! 💯
Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
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.
The NMS e2e test job is failing: https://github.com/magma/magma/runs/7498662277?check_suite_focus=true.
There are several warnings in the yarn test job about certain elements not used in the correct places. E.g. |
@@ -55,6 +55,13 @@ const useStyles = makeStyles<Theme>(theme => ({ | |||
fontWeight: 400, | |||
marginLeft: '8px', | |||
}, | |||
label: { | |||
fontSize: '14px', | |||
fontFamily: 'Inter', |
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 that fontFamily about?
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 looked a bit around and this is indeed a standard font that we use.
Although we typically use fontFamily: '"Inter", sans-serif',
This changes all labels across the app, but it does look better and it consistent with outer places so I'm fine with this change.
padding: '0 24px', | ||
}, | ||
login: { | ||
marginTop: '10%', | ||
}, | ||
title: { |
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.
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.
it was when I used the Grid component but now I need to keep most of it
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 see, this is because the title and subtitle are spans :|
a089b66
to
444b135
Compare
They still appear in the last job. 🕵️ |
I fixed some errors that are unrelated to login here #13430 |
@@ -55,6 +55,13 @@ const useStyles = makeStyles<Theme>(theme => ({ | |||
fontWeight: 400, | |||
marginLeft: '8px', | |||
}, | |||
label: { | |||
fontSize: '14px', | |||
fontFamily: 'Inter', |
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 looked a bit around and this is indeed a standard font that we use.
Although we typically use fontFamily: '"Inter", sans-serif',
This changes all labels across the app, but it does look better and it consistent with outer places so I'm fine with this change.
Signed-off-by: HannaFar <hannafarag159@gmail.com>
Signed-off-by: HannaFar <hannafarag159@gmail.com>
Signed-off-by: HannaFar hannafarag159@gmail.com
Summary
Add new style to the Login page
Organization portal sso enabled:
Organization portal:
Host portal:
Test Plan
Additional Information