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

add support for different logo variants #119

Merged
merged 7 commits into from
Oct 25, 2024

Conversation

gdfreitas
Copy link
Contributor

@gdfreitas gdfreitas commented Oct 23, 2024

Feature

Introduces a logoVariant property that accepts:

top (default)

image

bottom

image

legend.

In our use case would like use legend.

image

What do you guys think?

@gdfreitas gdfreitas marked this pull request as ready for review October 23, 2024 20:52
@eatyourpeas
Copy link
Member

Bumping this decision to @mbarton : possibly need a copyright/TM logo here next to RCPCH. Not also sure if having the logo present in the 'powered by' statement.

@mbarton
Copy link
Member

mbarton commented Oct 25, 2024

I don't know what decision making process in the college would decide where the logo should be and if it's required. So to avoid any unnecessary delays in your testing I would suggest we go with "Powered by RCPCH Digital Growth Charts - Version XXX" and no logo.

@gdfreitas
Copy link
Contributor Author

Hi @mbarton @eatyourpeas I updated the text according to your feedback. Thanks.

@gdfreitas
Copy link
Contributor Author

Hi @mbarton what do you think about using . instead of - ?

From Powered by RCPCH Digital Growth Charts - Version XXX to Powered by RCPCH Digital Growth Charts. Version XXX

I can do the change if you agree.

@eatyourpeas
Copy link
Member

Hi @gdfreitas I hope you don't mind me pushing to this PR. I think this is great and thank you for you time and thought putting it together. The extra bits I have added:

  1. fixed the broken tests
  2. added a dependency for storybook that must have fallen out with my last update to live (you will see I have added prettier back in and fixed all the css parsing errors in the tests in a previous PR to live)
  3. added 2 more stories - these are really just to demonstrate your logo presets in the context of prematurity where the age correction toggle is visible.

bottom
image

legend
image

@eatyourpeas eatyourpeas merged commit 60f1c08 into rcpch:live Oct 25, 2024
1 check passed
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