Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Brand - new backgroundColour & logoColour props #2127

Merged
merged 9 commits into from
Sep 16, 2019
Merged

Brand - new backgroundColour & logoColour props #2127

merged 9 commits into from
Sep 16, 2019

Conversation

sareh
Copy link
Contributor

@sareh sareh commented Sep 13, 2019

No ticket
A new brand colour will be needed shortly: #2120

We could build a theming object and pass that in, but in the case of Brand it's simpler to pass in just these two props instead.

Overall change: Update brand to take in colour props backgroundColour & logoColour. These are required props since there is no default colour, so this is a major version increase.

Code changes:

  • Update brand to take in colour props backgroundColour & logoColour
  • Add color storybook addon knob to change backgroundColour & logoColour

Storybook with standard colours:
Screenshot of brand component in Storybook

Can change the colours using the Storybook color add-on knob
Screenshot of brand component in Storybook, with backgroundColour changed

Localhost Storybook link: http://localhost:8180/?path=/story/components-brand--with-brand-link


  • I have assigned myself to this PR and the corresponding issues
  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@sareh sareh added ws-articles Tasks for the WS Articles Team shared-components labels Sep 13, 2019
@sareh sareh self-assigned this Sep 13, 2019
@@ -57,6 +62,8 @@ const Header = (product, serviceName) => (
minWidth={180}
svg={igbo}
url="https://www.bbc.co.uk/news"
backgroundColour={backgroundColour}
logoColour={logoColour}
Copy link
Contributor

@DenisHdz DenisHdz Sep 16, 2019

Choose a reason for hiding this comment

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

LGTM, but what about adding the color addon so we can also change the colors from the Storybook?

import {  color } from '@storybook/addon-knobs';
const backgroundColour = color('POSTBOX', `${C_POSTBOX}`);
const logoColour = color('WHITE', `${C_WHITE}`);

color-addon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Have updated it. I used the labels Background colour and Logo colour to keep them in keeping with the other knob labels that refer to the prop name.

@sareh sareh requested a review from DenisHdz September 16, 2019 08:56
Copy link
Contributor

@DenisHdz DenisHdz left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Contributor

@EinsteinNjoroge EinsteinNjoroge left a comment

Choose a reason for hiding this comment

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

LGTM

@staylos92
Copy link
Contributor

Happy for this to be merged 👍

@sareh sareh merged commit b077e58 into latest Sep 16, 2019
@sareh sareh deleted the brand-theming branch September 16, 2019 11:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
shared-components ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants