Skip to content

Conversation

@felix-bohlin
Copy link

I'm not sure if the tests are used anymore, they are missing some stuff so I could remove that part of the PR (or fix them) - I didn't put much work into it before checking with you.

Step 2 in this PR is for sure the documentation, or it could be a completely new PR.

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Owner

@argyleink argyleink left a comment

Choose a reason for hiding this comment

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

❤️ thanks for this!

you've got the files in there, and the tests look like they'll pass, but the one forgotten part was to add them to the main bundle and main JS object.


test('Should have an all included import', t => {
t.is(Object.keys(OpenProps).length, 1778)
t.is(Object.keys(OpenProps).length, 1816)
Copy link
Owner

Choose a reason for hiding this comment

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

main bundle doesnt include the new props because they're not in here https://github.com/argyleink/open-props/blob/main/build/props.js#L34

t.assert(OpenProps.orange0)
})

test('Import should have palette', async t => {
Copy link
Owner

Choose a reason for hiding this comment

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

same with this test, it'll pass once the palette props are part of the JS main bundle

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