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

fix: Process two-value shorthands appropriately #665

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

mellyeliu
Copy link
Member

Context

Previously we were indiscriminately expanding all multi-value shorthands to all four directions. Let's clean this up and split two-value shorthands like:

  • margin: '10em 1em' -> marginInline: '10em' and marginBlock: '1em'
  • padding: '10em 1em' -> paddingInline: '10em' and paddingBlock: '1em'

Implementation

Check for number of values after splitting:

  • If 1, return no error as before
  • If 2, return inline and block values
  • Else, return four directions

Testing

Added in test cases to ensure that two-value shorthands are accounted for.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 15, 2024
@mellyeliu mellyeliu requested review from necolas and nmn August 15, 2024 23:33
Copy link

compressed-size: runtime library

Size change: 0.00 kB
Total size: 2.52 kB

View unchanged
Filename: gzip (minify) kB size kB change % change
./packages/stylex/lib/stylex.js 1.04 (3.22) 0.00 (0.00) 0.0% (0.0%)
./packages/stylex/lib/StyleXSheet.js 1.48 (3.75) 0.00 (0.00) 0.0% (0.0%)

Copy link

compressed-size: e2e bundles

Size change: 0.00 kB
Total size: 1125.68 kB

View unchanged
Filename: gzip (minify) kB size kB change % change
./apps/rollup-example/.build/bundle.js 1002.49 (10185.35) 0.00 (0.00) 0.0% (0.0%)
./apps/rollup-example/.build/stylex.css 123.19 (774.43) 0.00 (0.00) 0.0% (0.0%)

@mellyeliu mellyeliu merged commit da2f65d into valid-shorthands-spacing Aug 15, 2024
9 checks passed
@mellyeliu mellyeliu deleted the two-value-shorthands branch August 15, 2024 23:47
@mellyeliu mellyeliu restored the two-value-shorthands branch August 16, 2024 00:06
mellyeliu added a commit that referenced this pull request Aug 16, 2024
@mellyeliu mellyeliu deleted the two-value-shorthands branch August 16, 2024 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants