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 responsiveArray #123

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sebtoombs
Copy link
Contributor

@sebtoombs sebtoombs commented Mar 7, 2023

DRAFT ONLY

Description

This PR adds support for the responsiveArray property. This property is supported by Sprinkles, and allows conditional properties to be provided to the atoms function as an array instead of only as an object.

This is achieved by optionally accepting the responsiveArray in defineProperties. The responsive array is then added as a top level property in the object returned by defineProperties. This is consumed in the runtime function via a new utility (createNormalizeValueFn) which looks up the config for a given property to find the relevant responsive array and crafts the responsive object.

This particular approach is probably not the way I would have done it if I was building the library from scratch, but I wanted to avoid major refactors/be as non-invasive as possible. That is to say; I'm not wedded to the actual implementation.

For example, there are no properties outside of the config that are returned from defineProperties at the moment. I added the responsiveArray outside because it felt cheaper than shoe-horning it into each property instead, but that could be another approach.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

@changeset-bot
Copy link

changeset-bot bot commented Mar 7, 2023

⚠️ No Changeset found

Latest commit: 62f3ea1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@sebtoombs sebtoombs mentioned this pull request Mar 7, 2023
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.

None yet

1 participant