newscorp-ghfb / NewsKit

The NewsKit Design system
https://newskit.co.uk
Other
130 stars 14 forks source link

Update Tag component scenarios - Storybook #743

Closed jps closed 1 year ago

jps commented 1 year ago

Description

Updates to the Tag component scenarios- Storybook

Tasks/Steps

Notes (main differences to what we have currently live versus the designs)

Acceptance/Testing Criteria

Given I am in Storybook

When I view the Tag component pages

Then the pages will be updated as per the designs

And the pages will be within 1024x768 for Applitools snapshots.

Business Benefit/Value

Consistent Tag examples for consumers, aligning to the NK branding.

Supporting Information

Designs: NewsKit - Storybook Project - Feedback and Status

Dependencies

N/A

Review with design when ready.

GeriReid commented 1 year ago

Thanks @JohnTParsons, a few edits please:

All stories: Please change the labels (eg small, medium, large, enabled, disabled) to Body010, inkSubtle

Transitions: Sorry, we didn't give enough design detail on this - in the dark theme the text fails colour contrast on hover and colours are a little funky.

Could you please replace with

this should give enough contrast light and dark.

Image

Styling overrides: Please remove this example Image

Docs page: Please add in the top section with a link to the docs site - Text: The tag element is used as a way of classifying content, typically using keywords. Link: https://www.newskit.co.uk/components/tag/

Image

Rest looks good - let us know if you spot anything else @nathanparris Here's the PR

nathanparris commented 1 year ago

The issue is the labels is due to < StorybookSubHeading > being used.

Can you use < StorybookCase title="label" > like how other stories are structured. e.g. https://storybook.newskit.co.uk/?path=/story/components-button--story-button-size

JohnTParsons commented 1 year ago

@GeriReid @nathanparris Changes made and pushed

GeriReid commented 1 year ago

thanks for making the changes @JohnTParsons, all looks good to me. Can you please have a final review @nathanparris?

nathanparris commented 1 year ago

@JohnTParsons The styling overrides story is failing contrast in dark theme. Can you update the stylePreset to use this.

      tagCustomTeal: {
        base: {
          borderStyle: 'solid',
          borderColor: '{{colors.interfaceBrand020}}',
          borderWidth: '{{borders.borderWidth010}}',
          color: '{{colors.interfaceBrand020}}',
          iconColor: '{{colors.inkInverse}}',
          backgroundColor: '{{colors.transparent}}',
        },
        hover:{
          backgroundColor: '{{colors.interface010}}'
        },

And for some reason the overrides theme isn't loading in, so the decorators need updating (I think the context.name was causing the issue)

decorators: [
    (
      Story: StoryType,
      context: {name: string; globals: {backgrounds: {value: string}}},
    ) => (
      <ThemeProvider
        theme={createCustomThemeWithBaseThemeSwitch(
          context?.globals?.backgrounds?.value,
          tagCustomThemeObject,
          context?.name,
        )}
      >
        <Story />
      </ThemeProvider>
    ),
  ],
JohnTParsons commented 1 year ago

@nathanparris Thanks for that feedback. Changes have been pushed and redeployed.

nathanparris commented 1 year ago

Thanks @JohnTParsons, looks good

GeriReid commented 1 year ago

thanks both, good to move this one to review @JohnTParsons