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

[Bug]: invalid border shorthand with custom properties #1354

Closed
stefanprobst opened this issue Mar 10, 2022 · 2 comments · Fixed by #1356
Closed

[Bug]: invalid border shorthand with custom properties #1354

stefanprobst opened this issue Mar 10, 2022 · 2 comments · Fixed by #1356

Comments

@stefanprobst
Copy link

Describe the bug

the following:

h1 {
  --width: 8px 2px;
  border-width: var(--width);
  border-style: solid;
  border-color: hotpink;
}

results in (default preset):

h1{--width:8px 2px;border:var(--width) solid hotpink}

which is invalid.

Expected behaviour

when a custom property is encountered, don't collapse border properties into shorthand.

Steps to reproduce

  1. paste the above snippet in the cssnano playground
  2. click minimize

Version

whatever is currently on the cssnano playground, 5.1.0 probably

Preset

default

Environment

System:
    OS: Linux 5.13 KDE neon 5.24
    CPU: (4) x64 Intel(R) Core(TM) i5-6400T CPU @ 2.20GHz
    Memory: 19.96 GB / 31.30 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash

Package details

=> Found "postcss@8.4.5"

Additional context

No response

@stefanprobst
Copy link
Author

the "lite" preset seems to handle this correctly

@ludofischer
Copy link
Collaborator

Duplicate of #675

the "lite" preset seems to handle this correctly

The lite preset disables declaration merging completely. You can also disable this optimisation in the default preset.

ludofischer added a commit that referenced this issue Mar 11, 2022
Fix #1354
Fix #675

- postcss-merge-longhand first 'explodes' longhand declarations to the smallest
unit (for example border-width -> border-top-width).
If the declaration value contains a custom property, it is not possible
to safely explode width/size/color, as the custom property might be equivalent to multiple
space-separated values.

- this plugin lowercases property names as a side-effect of exploding,
so to preserve the old behaviour we lowercase the property that cannot be
exploded

- remove incorrect tests and add tests for new behaviour
ludofischer added a commit that referenced this issue Mar 11, 2022
Fix #1354
Fix #675

- postcss-merge-longhand first 'explodes' longhand declarations to the smallest
unit (for example border-width -> border-top-width).
If the declaration value contains a custom property, it is not possible
to safely explode width/size/color, as the custom property might be equivalent to multiple
space-separated values.

- this plugin lowercases property names as a side-effect of exploding,
so to preserve the old behaviour we lowercase the property that cannot be
exploded

- remove incorrect tests and add tests for new behaviour
ludofischer added a commit that referenced this issue Mar 11, 2022
Fix #1354
Fix #675

- postcss-merge-longhand first 'explodes' longhand declarations to the smallest
unit (for example border-width -> border-top-width).
If the declaration value contains a custom property, it is not possible
to safely explode width/size/color, as the custom property might be equivalent to multiple
space-separated values.

- this plugin lowercases property names as a side-effect of exploding,
so to preserve the old behaviour we lowercase the property that cannot be
exploded

- remove incorrect tests and add tests for new behaviour
@ludofischer ludofischer moved this from To do to Done in merge-longhand bugs Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging a pull request may close this issue.

2 participants