SHS-6000: Stanford login font link changes #1705
Conversation
There was a problem hiding this comment.
@mariannuar The fix works, but need to do a couple of things:
- The form_alter doesn't work because it's being executed before other form alter that adds the
samlelement to the form. Let's remove it because it's not needed anymore. - It doesn't make sense to have
su_humsci_gin_admin.portal.cssminimized, because Drupal will do that for us. Let's update the formatting of the file to use multiple lines and indentation, to make future editing easier.
…_humsci_gin_admin.portal.css
|
@callinmullaney Ready for review! |
There was a problem hiding this comment.
@mariannuar Nice work! The irrelevant form_alter is gone, CSS is no longer minified (because Drupal does this), and the login page matches the screenshot in the ticket. Approved!
|
@ahughes3 Confirmed this PR is ready for your review/approval. |
| @@ -1 +1,33 @@ | |||
| .content-header{background:transparent;text-align:center}.content-header,.layout-container{max-width:600px;margin:0 auto}.user-login-form h1{display:none}.simplesamlphp-auth-login-link{display:inline-block;font-size:32px;padding:30px 0;position:relative;color:#b1040e}.simplesamlphp-auth-login-link::after{content:'\f054';font-family:'Font Awesome 5 Free';font-weight:900;font-style:normal;font-variant:normal;text-rendering:auto;line-height:1;color:#9e9e9e;position:absolute;top:calc(50% - 10px);font-size:20px;right:-20px} | |||
| .content-header { | |||
There was a problem hiding this comment.
Shouldn't the compiled css minified?
There was a problem hiding this comment.
@pookmish This was Andres' recommendation as using un-minified CSS to debug styling issues locally much easier without needing to manage multiple CSS compiling profiles based on the environment.
Also, because production and stage config are already being managed via config_split they enable CSS aggregation $config['system.performance']['css']['preprocess'] = TRUE; which minifies the CSS for us. So no need to minify things before they are sent to Drupal to be minified (again) and aggregated into a single file.
There was a problem hiding this comment.
Ok, I didn't see the comment and I noticed the diff in the changes. Just making sure that it's intentional. 👍
READY FOR REVIEW
Summary
Fix Stanford login font link
Urgency
medium
Note
In the
hs_adminmodule there's a preprocess function in line 131 that check if there's a SAML form and adds a classsimplesamlphp-auth-login-link, but it's not working. Something in thestanford_samlauthis working different or I'm not sure, couldn't find what is causing that preprocess to not add that class. However, using thesamlauth-logindoesn't seem to be a problem.Steps to Test
/user/loginStanford Loginlooks like thisPR Checklist