Skip to content
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

Merged
merged 9 commits into from
Oct 29, 2021
Merged

Update branding for Palace. #14

merged 9 commits into from
Oct 29, 2021

Conversation

ray-lee
Copy link
Contributor

@ray-lee ray-lee commented Oct 28, 2021

Description

This updates branding and styling for The Palace Project, including:

  • Change "Circulation Manager" text to "Palace Collection Manager"
  • Change header color to black on white
  • Add Palace Collection Manager logo to header
  • Change font to Open Sans
  • Change footer color to black on Palace light blue
  • Change color of blue buttons and links to Palace dark blue(-ish. This actually uses a lighter shade of Palace dark blue, so that on hover, the color can darken to become Palace dark blue.)

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:

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

Copy link
Contributor

@tdilauro tdilauro left a 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.

@@ -34,6 +34,6 @@ export default class AccountPage extends React.Component<{}, {}> {
}

UNSAFE_componentWillMount() {
document.title = "Circulation Manager - Account";
document.title = "Palace Collection Manager - Account";
Copy link
Contributor

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).
@ray-lee
Copy link
Contributor Author

ray-lee commented Oct 29, 2021

@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.

@ray-lee ray-lee requested a review from tdilauro October 29, 2021 03:47
Copy link
Contributor

@tdilauro tdilauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants