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

Chore: Upgrade Skeleton to Chakra v3 #1515

Open
wants to merge 11 commits into
base: update-chakra
Choose a base branch
from
Open

Conversation

hunshamar
Copy link
Contributor

Background

Upgrading to chakra 3 breaks default styling applied, so created a recipe.

Solution

Chakra update checklist

  • Updated Sanity documentation in v2 dataset (English, links, component props and content)
  • Updated documentation in the component file
  • Update green-beans-check.md with any major changes
  • Add changeset
  • Double check design in Figma (no design for Skeleton)

UU checks

  • It is possible to use the keyboard to reach your changes
  • It is possible to enlarge the text 400% without losing functionality
  • It works on both mobile and desktop
  • It works in both Chrome, Safari and Firefox
  • It works with VoiceOver
  • There are no errors in aXe / SiteImprove-plugins / Wave
  • Sanity documentation has been / will be updated (if neccessary)

@hunshamar hunshamar requested review from ithen15 and a team as code owners February 20, 2025 11:47
@hunshamar hunshamar requested review from Cmoen11, leiferikbjorkli, cibietici, alicemacl, mtdyrli, mcklien and Siljeelisestrm and removed request for a team February 20, 2025 11:47
Copy link

changeset-bot bot commented Feb 20, 2025

🦋 Changeset detected

Latest commit: b4f44fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@vygruppen/spor-react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@alicemacl alicemacl left a comment

Choose a reason for hiding this comment

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

Noen få ting!

},
variant: {
pulse: {
background: "steel",
Copy link
Contributor

Choose a reason for hiding this comment

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

Vi må huske darkmode på denne, foreslår:

Suggested change
background: "steel",
background: {
_light: "silver",
_dark: "darkGrey",
},

shine: {
"--animate-from": "200%",
"--animate-to": "-200%",
"--start-color": "colors.lightGrey",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"--start-color": "colors.lightGrey",
"--start-color": {
_light: "colors.lightGrey",
_dark: "colors.dimGrey",
},

"--animate-from": "200%",
"--animate-to": "-200%",
"--start-color": "colors.lightGrey",
"--end-color": "colors.steel",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"--end-color": "colors.steel",
"--end-color": {
_light: "colors.platinum",
_dark: "colors.darkGrey",
},

Comment on lines +48 to +51
defaultVariants: {
variant: "pulse",
loading: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Dette må slettes. Det er en bug at andre varianter arver verdier fra default variant. Definer det heller i props

export const Skeleton = forwardRef<HTMLDivElement, SkeletonTextProps>(
function SkeletonText(props, ref) {
const recipe = useRecipe({ recipe: skeletonRecipe });
const [recipeProps, restProps] = recipe.splitVariantProps(props);
Copy link
Contributor

Choose a reason for hiding this comment

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

Usikker på om denne trengs, eller om du bare kan legge props i recipe?

return (
<Circle size={size} asChild ref={ref}>
<ChakraSkeleton {...rest} />
<ChakraSkeleton {...rest} loading={false} css={styles} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg får error på loading her, noe type mismatch

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