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

Fixes less/css compilation when using less.js v4 #860

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Jan 8, 2024

Description (*)

This complements magento/magento2#38335

In your internal tickets AC-8098 & AC-9713 less.js got upgraded from v3 to v4
However, that switch is not fully backwards compatible, the math stuff now needs to be encapsulated in parentheses for less.js to do the calculation and not output it as-is to the generated css.

This module only has one mistake as far as I could find, this is the output of compiling the less code like it is right now with v4 compared to v3 and this results in invalid css:

--- pub/static/adminhtml/Magento/backend/en_US/css/styles.css 2024-01-08 17:44:24
+++ pub/static/adminhtml/Magento/backend/en_US/css/styles.css  2024-01-08 17:44:34
@@ -13248,7 +13248,7 @@
 }
 [data-content-type='products'] .product-reviews-summary,
 .pagebuilder-products .product-reviews-summary {
-  margin-bottom: 1.5rem;
+  margin-bottom: 1rem + 0.5rem;
 }
 [data-content-type='products'] .product-reviews-summary .reviews-actions,
 .pagebuilder-products .product-reviews-summary .reviews-actions {

This PR fixes it and will output margin-bottom: 1.5rem; again.

Manual testing scenarios (*)

See testing scenario in magento/magento2#38335

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)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

@hostep
Copy link
Contributor Author

hostep commented Jan 8, 2024

@magento run all tests

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@m2-community-project m2-community-project bot moved this from Ready for Review to Reviewer Approved in Pull Request Progress Jan 9, 2024
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@hostep
Copy link
Contributor Author

hostep commented Jan 9, 2024

Is no longer needed, this was only needed when the strictMath option was enabled in the less.js compilation. But we'll keep it disabled. See magento/magento2#38335 (comment)

Closing, sorry for the noise!

@hostep hostep closed this Jan 9, 2024
@m2-community-project m2-community-project bot removed this from Reviewer Approved in Pull Request Progress Jan 9, 2024
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

1 similar comment
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

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

Successfully merging this pull request may close these issues.

None yet

2 participants