elchininet / postcss-rtlcss

PostCSS plugin to automatically build Cascading Style Sheets (CSS) with Left-To-Right (LTR) and Right-To-Left (RTL) rules using RTLCSS
https://elchininet.github.io/postcss-rtlcss/
Apache License 2.0
98 stars 14 forks source link

Support specifying processors during processing CSS declarations #327

Closed aleen42 closed 2 months ago

aleen42 commented 2 months ago

This pull request implements an option named processDeclarationPlugins to support stepping into the process of processing CSS declarations, like avoiding flipping background-positions.

For instance, RTLCSS will flip the background-position:

input

.test {
    background-position: 0 100%;
}
Convert 0 to 100% (default)

output

.test {
    background-position: 100% 100%;
}

However, we may need to stop this unexpected flipping in the case of using it to clip Sprite images, so we can use this introduced option to avoid this:

const options = {
    processDeclarationPlugins: [
        {
            name: 'avoid-flipping-background',
            priority: 99, // above the core RTLCSS plugin which has a priority value of 100
            processors: [{
                expr: /(background|object)(-position(-x)?|-image)?$/i,
                action: (prop, value) => ({prop, value})}
            ]
        }
    ]
};
output
.test {
    background-position: 0 100%;
}
elchininet commented 2 months ago

Hi @aleen42,

It is always recommended to discuss the intention of creating a feature before working on it, just to avoid working in vain.

If you want to support plugins and hooks, it cannot be done in that way that you did and you cannot link plugins and hooks of RTLCSS. It will create confussion and it will be a source of issues because most of the things supported by RTLCSS will not work with this plugin.

postcss-rtlcss uses RTLCSS behind the scenes, but this does not mean that it runs RTLCSS in the entire CSS input. RTLCSS is used only on declarations, but it doesn‘t run in any other element (the CSS root element, the rules, at rules, comments, etc). This means that anything that a user sets inside directives will potentially fail, I think that the only thing that will work without issues is the processors property.

The same with hooks, as RTLCSS is not executed in the CSS root, most probably what RTLCSS users expect from a hook will not work with postcss-rtlcss. So, if you want to build hooks, most probably it should be built here from scratch (the same way the control directives and the stringMap option are built from scratch).

Please, refactor this following the next:

Regards and thanks for contributing to the project.

coveralls commented 2 months ago

Coverage Status

coverage: 100.0%. remained the same when pulling dd78fd02c406bb9c8781732cf1ec61239863dd21 on aleen42:master into d220b6450f9bcb04ec77eb003ce74b470f24eed0 on elchininet:master.

aleen42 commented 2 months ago

@elchininet Thanks for your review. I have eliminated the unnecessary hooks options because it seems we do not need to hook during processing CSS declarations via postcss-rtlcss. The plugin options have been renamed as processDeclarationPlugins which are currently only applied during processing declarations.

elchininet commented 2 months ago

Hi @aleen42, Looks good to me. Update the branch (I think that you may get some merge conflicts). Regards

elchininet commented 2 months ago

@aleen42, I think that it is ready to merge. Thanks for your contribution. I'll release a new version when I have a chance. Regards 👍🏼

elchininet commented 2 months ago

Hi @aleen42, The feature has been released in version 5.3.0. Regards