csstools / stylelint-use-logical

Enforce usage of logical properties and values in CSS
Creative Commons Zero v1.0 Universal
59 stars 9 forks source link

Except doesn't work as expected #3

Closed yepninja closed 7 months ago

yepninja commented 5 years ago

Version of stylelint-use-logical

1.1.0

Config

module.exports = {
    plugins: [
        'stylelint-use-logical',
    ],
    rules: {
        'csstools/use-logical': ['always', {
            except: [
                'top',
                'left',
            ],
        }],
    },
}

Code

/* index.css */
.selector {
    position: absolute;
    top: -4px;
    left: 0;
}

Error

index.css 11:2 ✖ Unexpected top property. Use inset-start. csstools/use-logical

Expected

No error because top property is added to the expect array.

alehar9320 commented 4 years ago

I can second this. I cloned the repository and put in the following test scenario into the .tape.js to verify the issue.

Test case

{ source: 'body { margin-top: 0.5rem; margin-bottom: 0.5rem; }', expect: 'body { margin-top: 0.5rem; margin-bottom: 0.5rem; }', args: ['always', { except: ["margin-top", "margin-bottom"] }], warnings: 0 }

Reported result:

× body { margin-bottom: 0.5rem; margin-top: 0.5rem; } with "always", { "except": [ "margin-top", "margin-bottom" ] }  becomes body { margin-bottom: 0.5rem; margin-top: 0.5rem; }
  Expected: body { margin-bottom: 0.5rem; margin-top: 0.5rem; } **Recieved: body { margin-block: 0.5rem; }**

It appears to be the case that it does a prop2 replacement, despite that they're both listed as exemptions.

Reason for error

Inside the code we map prop2 and prop4 conversion. These are not covered by the except.

Proposal for solution

Add checks for prop2 and prop4. If any of the except properties are identified, skip prop2/prop4 conversion and instead rely on one-to-one conversion.

Bonus, if prop2 and prop4 has feature flags and may be opted out of through configuration.

jens-duttke commented 4 years ago

I have the same problem using

'csstools/use-logical': ['always', { except: ['top', 'bottom', 'margin-top', 'margin-bottom', 'padding-top', 'padding-bottom'] }],

and still getting

Unexpected top property. Use inset-start. (csstools/use-logical) [error]
Unexpected margin-top property. Use margin-block. (csstools/use-logical) [error]

This issue is more than one year old, just like a pull request from October last year. Can we assume that this plugin is no longer being developed?

Jordan-Hall commented 3 years ago

@jens-duttke I've started fixing the bugs and ensure its too standard over here https://github.com/Jordan-Hall/stylelint-use-logical-spec. I'll link this issue so i can resolve it

vmohir commented 1 year ago

I have kind of the same problem.

My config:

module.exports = {
  plugins: ['stylelint-use-logical'],
  customSyntax: 'postcss-less',
  rules: {
    'csstools/use-logical': [
      'always',
      {
        except: [
          'top',
          'bottom',
          /^(padding|margin|border)-(top|bottom)/,
          /width$/,
          /height$/,
        ],
      },
    ],
  },
};

The style:

padding-top: 10px;
padding-bottom: 10px;

This is raising an error. but this style is not:

padding-top: 10px;

So it seems having both of the padding-top and padding-bottom is not supported by the except property.

romainmenke commented 7 months ago

This has been fixed. Thank you for reporting