Fix some nested extends have been ignored at output

[mrtuvn]

Description (*)

Difference being that .abs- styles which extend other .abs- styles in _extends.less are not output properly when compiled with Grunt, due to being nested (e.g. .abs-action-addto-product, which extends .abs-action-link-button or .abs-account-blocks). This creates differences between production and development sites, which make some style issues invisible in a standard development workflow.

More details provide
E.g. customer account page
https://github.com/magento/magento2/blob/2.4-develop/app/design/frontend/Magento/blank/Magento_Customer/web/css/source/_module.less#L136
call .abs-account-blocks in module.less,

.abs-account-blocks defined in _extends.less call to .abs-account-title
https://github.com/magento/magento2/blob/2.4-develop/app/design/frontend/Magento/blank/web/css/source/_extends.less#L197

So the results output we will miss some styles. Reason see link below describe in less docs

From less doc describe extending does not match selectors inside a nested @media declaration:
Reference => http://lesscss.org/features/#extend-feature (open in new tab)

Less official doc
Link http://lesscss.org/features/#import-atrules-feature (open new tab)

Before change tests

Less compile to css by php

Use grunt/lessc
Magento use grunt + grunt-contrib-less for compile

We lost some rules styles block-title

RESULTS:

After change
Both workflow output same results (php less compile vs grunt or client less compiler)

Before remove (reference)
styles-m.css 375,4 kB 14669LOC
styles-l.css 95,1 kB 3685LOC

After Remove (reference)
styles-m.css 375,4 kB 14669LOC
styles-l.css 95,1 kB 3685LOC

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes Different results from compiling less with Grunt or with "server side compilation" #7231

Manual testing scenarios (*)

1.Create a custom theme that extends Magento/blank or Magento/luma
2.Create an _extend.less file in the custom theme
3.Add various styles to each group with the same selectors and specificity
4.Recompile CSS with either Grunt or server-side compilation
5.Confirm that all common styles in styles-m.css (including those from _extend.less) are output above all media queries

Scenario1 Server-side less compilation
Normal magento use by run commands
setup:upgrade && setup:static-content:deploy
https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/css-guide/css_quick_guide_mode.html

Scenario2 Grunt compilation
https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/css-topics/css_debug.html

Scenario 3 Less frontend compilation (backend config)

1. Enable developer mode (lessjs compile not work in production)
2. Enable Client side compilation in Advanced Configuration
   (Stores > Configuration > Advanced > Developer
   Frontend Development Workflow => Workflow type : Client side less compilation)
3. Save and clear cache config admin

Questions or comments

CC: @Leland @krzksz @ptylek

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @mrtuvn. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[gabrieldagama]

@magento run all tests

[ihor-sviziev]

I'm pretty sure functional and web API tests can't be failing because of CSS changes. Also, test results look not related
@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE, WebAPI Tests

[ihor-sviziev]

✔ Approved.

Failing tests looks not related to changes form this PR.

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8684 has been created to process this Pull Request

[mrtuvn]

For compatible i don't bring media-common mixin to blank theme only apply for luma and extends less file both themes

[mrtuvn]

@magento run all tests

[ihor-sviziev]

Please review and fix the failing static tests

[mrtuvn]

I think new difference (Server Side Compilation) can acceptable as main archivement we want both workflow should visual same while compare with grunt or less frontend compilation. Some extend class not available in previously version
Before this change i think they not be the same. If we run by grunt we can't trust the final results. When using grunt we're missing some the nested &:extend(…) in &:extend(…) and we can't trust the CSS we're seeing. They difference than result by php less compile. What do you guys think?

[ihor-sviziev]

@engcom-Alfa, I checked the diff between the mainline and with a fix - seems like that was an issue that some styles were missing.
✔ approved from my side

[engcom-Alfa]

✔️ QA Passed

Case 1. Grunt compilation

Manual testing scenario:

1. Create a custom theme that extends Magento/luma;
2. Create an _extend.less file in the custom theme;
3. Add various styles to each group with the same selectors and specificity;
4. Recompile CSS with either Grunt;

Before: ✖️ some block-title rule styles are missing

After: ✔️ All styles are applied

Case 2. Server-side less compilation

Manual testing scenario:

1. Normal magento use by run commands
2. Run: setup:upgrade && setup:static-content:deploy
   https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/css-guide/css_quick_guide_mode.html

After: ✔️ All styles are applied

Case 3. Less frontend compilation (backend config)

1. Enable developer mode (lessjs compile not work in production);
2. Enable Client side compilation in Advanced Configuration
   (Stores > Configuration > Advanced > Developer
   Frontend Development Workflow => Workflow type : Client side less compilation);
3. Save and clear cache config admin;

After: ✔️ All styles are applied

✔️ No differences were found with the different compilation types

• Server Side Compilation (with fix) & Grunt (with fix) styles-m - ✔️ No difference found (diff)
• Server Side Compilation (with fix) & Grunt (with fix) styles-l - ✔️ No difference found (diff)

The styles have been checked on different pages with samples and everything is working properly.
And on mobile browsing also everything works well.

[m2-assistant]

Hi @mrtuvn, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[serhii-chernenko]

hey guys

General

Selectors of referenced files of styles shouldn't be included to output css files. This fix brakes previous logic.
Let me explain my point.

Env

Build: default deploying (setup:static-content:deploy)
Theme: clear theme which extends Magento Blank Theme

Before

When we had the directive reference for _extend.less files:

We had 194 matches of abs selectors which extended in styles-m.css file

And 36 matches in styles-l.css:

styles-m.css had size: 53.2kb.

After

When this fix has been applied and the directive reference was removed, we have different result.

Fix:

We have 323 matches of abs selectors which NOT ONLY extended in styles-m.css file

And 45 matches in styles-l.css:

styles-m.css have size: 53.9kb (was 53.2.kb)

Explanation

Selectors of referenced files of styles shouldn't be included to output css files. This fix brakes previous logic.

With reference (before this commit)

If I move _extend.less file to new clear theme and write my own abs selector .abs-custom-bg but without using it:

I don't have any matches with the selector:

Even I extend the selector for any selectors in any less file without the directive reference, e.g. _extend.less:

I don't have matches with the abs selector but I have applied styles via &:extend:

Without reference (after this commit)

I write my own abs selector without using it:

But this selector compiled to output css as simple selector of any less files:

And when I extend it in any files:

I have both selectors:

Summary

The logic of creating helper selectors have been destroyed to use abs selectors (not all) when you need it in any files but not compile ALL abs selectors to the output css file.

Most of production projects don't use Grunt as a tool of deploying styles. It has to work with the default deploying process of magento as expected. Please rollback changes.

cc: @ihor-sviziev

[ihor-sviziev]

@mrtuvn could you please look at @Inevix comment?

[serhii-chernenko]

@ihor-sviziev @mrtuvn
I've investigated the issue which has to be fixed in this PR and I have some points and solution for you.

Points

1. Selectors in referenced files expect that they have to be extended in OTHER NOT REFERENCED files. Don't use extend in the same referenced file.
2. Grunt doesn't compile extend selectors even without wrappers, such as: media-common or media mixins.
3. Before making some fix, see any examples which already exist in this file or in any same files.
4. Use mixin instead of extend in the referenced file.

Example

Right way:

Wrong way:

Solution

Add reference to import of _extends.less.

Imports of the file really might be removed from styles-m and styles-l files:

Change the code of extend for .abs-block-title:

To a new mixin:

Result

Default deploy:

Grunt:

[ihor-sviziev]

@Inevix, if you already did research, could you please send a Pull Request with suggested changes?
Thank you!

[serhii-chernenko]

@Inevix, if you already did research, could you please send a Pull Request with suggested changes? Thank you!

@ihor-sviziev done:
#35101