Skip to content

S1 style macro#6009

Merged
devongovett merged 7 commits intomainfrom
s1-macro
Mar 6, 2024
Merged

S1 style macro#6009
devongovett merged 7 commits intomainfrom
s1-macro

Conversation

@devongovett
Copy link
Copy Markdown
Member

This implements a macro to generate atomic spectrum styles at build time for Spectrum 1. It maps to our existing CSS variables so that the values change according to Provider settings like color scheme and scale. The package is currently intended to be a private implementation detail of our components but we might expose it at some point.

In order to do this, I also updated Parcel to v2.12 and typescript to 5.3.

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 5, 2024

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 5, 2024

Copy link
Copy Markdown
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

super nit that all the copyrights need to be updated

@devongovett
Copy link
Copy Markdown
Member Author

🙈 Do they though? Haha we never update our other ones.

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 6, 2024

@rspbot
Copy link
Copy Markdown

rspbot commented Mar 6, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

Copy link
Copy Markdown
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Haha, I think we just do it when we notice. Though it really doesn't make much sense to me.
Anyhow, it looks pretty good, verified the story works. I like that you've managed to tie in some of the values with the actual variables

Copy link
Copy Markdown
Collaborator

@ktabors ktabors left a comment

Choose a reason for hiding this comment

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

This is just for limited user while we work on docs, right?

{
"name": "@react-spectrum/style-macro-s1",
"private": true,
"version": "3.5.3",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

alpha release?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's private so won't be published for now. also versions get updated when we do releases.

aria-label="Choose frequency"
selectionMode="single"
width="size-1200">
<></>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is this change for?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

TypeScript complained that children were required but there were none here.


export function Example() {
return (
<div className={style({backgroundColor: 'orange-500', color: 'black', fontSize: 'lg', paddingX: 8, paddingY: 4, borderRadius: 'default'})()}>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need an example passing in variables that change, like hover, to show how that works and to see that it works?

return `[light-dark(${color[light]}, ${color[dark]})]`;
}

const baseSpacing = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we switch this to pixel instead of 4px grid?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think we have made that decision yet. This will all get updated

const sizing = {
...scaledSpacing,
auto: 'auto',
'1/2': '50%',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we remove factions and people use percentages directly like [50%]?

'4/6': '66.666667%',
'5/6': '83.333333%',
full: '100%',
screen: '100vh',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Did we want to add 100wh?

@devongovett devongovett merged commit 58872ae into main Mar 6, 2024
@devongovett devongovett deleted the s1-macro branch March 6, 2024 15:12
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.

4 participants