newscorp-ghfb / NewsKit

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

Update Link component scenarios - Storybook #742

Closed jps closed 1 year ago

jps commented 1 year ago

Description

Updates to the Link 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 Link component pages Then the pages will be updated as per the designs

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

Review with design when ready.

Business Benefit/Value

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

Supporting Information

Designs: NewsKit - Storybook Project - Feedback and Status

mstuartf commented 1 year ago

@GeriReid @nathanparris this is deploying and ready for review.

A few issues:

  1. The outline star icon is coming through as filled. I think this is an issue with emotion, rather than something on our side. Is it ok as it is?
  2. I'm not sure that there's much value to the last scenario. The cases either repeat previous scenarios or aren't really demonstrating the link functionality (e.g. custom width).

Thanks

GeriReid commented 1 year ago

Hey @mstuartf

  1. Filled icon is fine
  2. Agree - omit the last story

Do you have a PR link we can review? thanks

mstuartf commented 1 year ago

Sorry, I thought the issue would link to the PR.

Here you go: https://github.com/newscorp-ghfb/NewsKit/pull/797

GeriReid commented 1 year ago

thanks @mstuartf, looks good. There's a contrast fail on the dark theme:

Sorry, this is down to us not speccing it. Is it possible to use the same style preset as the overrides story?

mstuartf commented 1 year ago

Yeah I thought that might be an issue. At the moment the custom theme is only available in stories with the name Styling overrides. There's a ticket to make that more flexible in the backlog.

Do we need the styling overrides cases in the paragraph story if we have a separate Styling overrides story already?

GeriReid commented 1 year ago

@mstuartf that's a good point - we demonstrate the overrides in the Styling overrides story. Suggest we omit them from theInline link in paragraph story, will save some hassle!

mstuartf commented 1 year ago

Thanks, @GeriReid. I've remove those sections - will move to peer review.