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

Color Rebranding with Digital Grey and Yellow #trivial #421

Merged
merged 25 commits into from
Sep 14, 2023

Conversation

200455939-yashu
Copy link
Collaborator

Fixes # .

Changes proposed in this PR: The Rebranding changes related to the color are added

#331

@github-actions
Copy link

github-actions bot commented Aug 16, 2023

2 Warnings
⚠️ There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior.
⚠️ This PR is too big! Consider breaking it down into smaller PRs.

Generated by 🚫 Danger

@200455939-yashu 200455939-yashu changed the title Color Rebranding with Digital Grey and Yellow Color Rebranding with Digital Grey and Yellow #trivial Aug 16, 2023
aaronskiba

This comment was marked as duplicate.

Copy link
Collaborator

@aaronskiba aaronskiba Sep 7, 2023

Choose a reason for hiding this comment

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

I'm wondering about the various .png and .svg files with respect to GB:

  • For all of the en-CA, en_CA, fr-CA, and fr_CA files, there is a .png as well as a corresponding .svg file. Should we also have app/assets/images/dmp_logo_en-GB.svg and app/assets/images/dmp_logo_en_GB.svg? (Or maybe we don't need the GB files at all?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need the British English logo, but currently lack the SVG files. @lagoan, Please assist!

app/assets/stylesheets/application.scss Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

You mention the formatting of this stylesheet into multiple stylesheets. Not sure if we should handle this now or later @lagoan? If later, maybe just make a GitHub issue for it in the meantime.

Also, there is just one comment typo Drop downs of the coloege lists in create plans section that is a bit difficult to decipher.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

About the formatting the stylesheet this will be easier to track the changes as well as it will easier to add or modify these changes.
@lagoan please guide us.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A branding page exists on the DMPRoadmap wiki, and includes a recommended approach for stylesheet customizations: https://github.com/DMPRoadmap/roadmap/wiki/Branding#stylesheets

Maybe this is the best way to refactor, @lagoan?

Copy link
Collaborator

@aaronskiba aaronskiba left a comment

Choose a reason for hiding this comment

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

Approved. Also, please create Issues for the following:

  • Refactoring of these changes
  • Missing Sandbox logo

@200455939-yashu 200455939-yashu merged commit 0981238 into deployment-portage Sep 14, 2023
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