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

Except doesn't work as expected #3

Closed
yepninja opened this issue Jul 29, 2019 · 5 comments
Closed

Except doesn't work as expected #3

yepninja opened this issue Jul 29, 2019 · 5 comments

Comments

@yepninja
Copy link

yepninja commented Jul 29, 2019

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
Copy link

alehar9320 commented Jan 14, 2020

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
Copy link

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
Copy link
Contributor

@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
Copy link

vmohir commented Jun 1, 2023

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
Copy link
Member

This has been fixed.
Thank you for reporting

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

No branches or pull requests

6 participants