Updates MIT branding#57
Conversation
Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/PW-68 How does this address that need: * Updates MIT logo * Updates favicons * Moves MITL wordmark and background to CDN loaded assets * Removes social media links * Adjust some alignment Document any side effects to this change: * Removed an IE8 and older shim from being loaded
Pull Request Test Coverage Report for Build 7170810188
💛 - Coveralls |
matt-bernhardt
left a comment
matt-bernhardt
left a comment
There was a problem hiding this comment.
The only change that I think needs to be made is to remove the alt attribute from the link in the libraries' footer - that's getting flagged by ANDI across many of our platforms.
Everything else in here I'm comfortable with so am not requesting changes - but there are a few things I wonder whether we're going to hear from UX about when they review everything:
- The size of the logos in the footer, when we did them for Omeka, were set to match each other's widths (since the two logos appear above one another, that was determined to be the relevant size). This isn't noted this way in the slides, though, so maybe they've moved away from that thinking by this point.
- The padding above/below logos on both the header and footer are called out to be 20px, which isn't always the case in our current implementations. This isn't a new change you're making here, and I'm not sure whether we're being too careful on this point in trying to hit exactly that size.
| <img | ||
| src="https://cdn.libraries.mit.edu/files/branding/local/mit_lockup_std-three-line_rgb_white.svg" | ||
| alt="MIT logo" | ||
| height="35" |
There was a problem hiding this comment.
I see this constraint in the footer, and had to go back to the slides which UX shared with us because I remembered the footer logo being constrained differently. They didn't annotate the footer in the same way that the conversation around Omeka went (where the footer logos were constrained to match widths with each other, because they're appearing one atop the other).
In terms of this PR, I'm going to say this is fine - but I wonder whether they're going to ask about it.
There was a problem hiding this comment.
Yeah, considering we have 4 differently implementations that will be based on the slides, I think I'll leave this as-is and let Darcy clarify in the slides/feedback if the requirements are different than they were conveyed. Thanks for the heads-up though as I'll make sure to explicitly ask her about this.
2 participants
Note: this has been deployed to a PR build of the TACOS app to simplify assessing the changes being made:
https://tacos-api-pipeline-pr-22.herokuapp.com
Relevant ticket(s):
How does this address that need:
Document any side effects to this change:
Developer
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Code Reviewer
(not just this pull request message)
Requires database migrations?
NO
Includes new or updated dependencies?
YES