-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update branding for Palace. #14
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.
This generally looks good. One comment about the duplication of the app name in a lot of places.
src/components/AccountPage.tsx
Outdated
@@ -34,6 +34,6 @@ export default class AccountPage extends React.Component<{}, {}> { | |||
} | |||
|
|||
UNSAFE_componentWillMount() { | |||
document.title = "Circulation Manager - Account"; | |||
document.title = "Palace Collection Manager - Account"; |
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.
"Palace Collection Manager" is used all over the place now. Wondering if it might be useful to add this value through a higher-level context provider (or co-opt/rename one of the existing ones) as appName
or similar and then reference it where needed.
We could use a similar approach and parameterize/interpolate in tests and maybe even SCSS, if needed.
…nent. Also publish the logo to the dist directory so it can be loaded by other apps (via jsdelivr).
@tdilauro I moved the title formatting into a function to remove the duplication. There are now just a couple occurrences of "Palace Collection Manager" outside of that function, in the browser automation tests, which were written in a way that makes it hard to import modules in the normal way. I think we're just going to delete these tests at some point, because we're using the A1QA test suite to do this. I also changed the logo to be a normal (svg) image instead of a React component, which gets published to the dist directory, and will be available through jsdelivr. This will allow the CM python app to load it in HTML pages it generates, the same way it uses the js and css bundles. |
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.
Looks good! 🚀
Description
This updates branding and styling for The Palace Project, including:
The Notion ticket asked for the red buttons to become Palace logo red, but I found that Palace logo red is obnoxiously, shockingly, distractingly bright when used on buttons. I think the existing darker red is fine.
Screenshots:
Motivation and Context
This makes the CM front-end follow Palace branding guidelines, and differentiates the Palace CM from SimplyE.
Notion: https://www.notion.so/lyrasis/Update-CM-with-Brand-guidance-88976ea09252447998779b4bbeb2fdeb
How Has This Been Tested?
@ray-lee browsed around the UI in Chrome and Firefox.
Checklist: