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 a link card component based on Survey of Londoners #145

Merged
merged 12 commits into from
Oct 18, 2023
Merged

Conversation

jamesscottbrown
Copy link
Collaborator

@jamesscottbrown jamesscottbrown commented Oct 12, 2023

What does this change?

This adds a Card component that is acts as a link.

Why?

This pattern has been used in a few places so far (e.g., Survey of Londoners and Wellbeing Explorer), and I want to use it on the multivariate census explorer too,

Does this introduce new dependencies?

No.

How is it tested?

How is it documented?

Are light and dark themes considered?

Is it complete?

  • Have you included changeset file?
  • If this adds a new component, is it exported via index.js?

@ChrisKnightLDN
Copy link
Contributor

This looks fine, though there are a couple of concerns: Naming - there is an existing proto design for a card component that is more consistent with https://open-ui.org/components/card.research/ the component added in this pr is more of a CTA button(?) - though that isn't a suggestion for the name - we can discuss. Styling - the styles applied in the wellbeing project shouldn't be followed really as it's a unicorn. So the rounded corners are probably redundant. Designs will need to be finalised.

Copy link
Contributor

@ChrisKnightLDN ChrisKnightLDN left a comment

Choose a reason for hiding this comment

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

Looks good to me

@ChrisKnightLDN ChrisKnightLDN merged commit 1713d79 into dev Oct 18, 2023
1 check passed
@ChrisKnightLDN ChrisKnightLDN deleted the card-link branch October 18, 2023 08:46
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.

2 participants