rcpch / digital-growth-charts-react-component-library

A typescript React library for displaying RCPCH Digital Growth Charts from API data
https://growth.rcpch.ac.uk
MIT License
8 stars 11 forks source link

Text Styles Customization #111

Open gdfreitas opened 1 month ago

gdfreitas commented 1 month ago

Hi everyone,

I noticed two situations related to customizing styles, particularly with fonts:

  1. Texts style property style="bold" does not seem to have any effect. Not sure, but I believe it tries to apply font-weight values to a font-style CSS property, which is not valid, perhaps we could have a separate weight property for that.

  2. Would be nice to have API for customization of some texts:

    • delayedPubertyThresholdLabel.fontSize
    • centileLabel.fontSize | fontSize.fontFamily

I got the references from src/functions/makeAllStyles.ts

eatyourpeas commented 1 month ago

Thanks @gdfreitas great suggestion. Fonts have proved a little problematic and in the end the easiest way to include fonts in the react build in the end was to b64 encode them. Currently therefore there is only montserrat in there, as that is the RCPCH standard. If you want other fonts we would be very open to you adding them. In the earlier versions we gave more freedom to users to changes some styles and this was deprecated in 7.0 in favour with theming, though you can still create a custom theme. If the weight has not made it I am open to the idea of adding it? There is a TextStyle interface - are you interested in trying to implement something that works and send in a PR?

gdfreitas commented 1 month ago

Hi @eatyourpeas ,

I would love to contribute to, I'm actually already working on it locally on a branch.

Could you review if I have the right permissions for pushing? I'm getting this error

remote: Permission to rcpch/digital-growth-charts-react-component-library.git denied to gdfreitas. fatal: unable to access 'https://github.com/rcpch/digital-growth-charts-react-component-library.git/': The requested URL returned error: 403

eatyourpeas commented 1 month ago

Sure @gdfreitas I don't think you will have permissions to push direct to the repository. Might be best if it is ok for you to fork the repository and when you have some code you want us to look at, just send in a PR and we will look at it. Very grateful for your input and expertise - and your avatar will appear on our repo too :)

The makeAllStyles function gets called at instantiation of the component, and this sets all the base styles and defaults. If users are happy with one of the themes we offer out of the box, you can just pass this in using the themes prop. Not all items can be overridden as some standards were set by the growth chart project board. Lines, for example, can be any colour (within reason) but must be one colour, not multiple colours.

eatyourpeas commented 1 month ago

Leaving this item open because although this has resolved in part a wider discussion was had in the PR on whether fonts should be bundled with the library or whether implementers should be encouraged to implement them separately.