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

Less code isn't 100% valid #775

Closed
hostep opened this issue Sep 6, 2021 · 4 comments · May be fixed by #777
Closed

Less code isn't 100% valid #775

hostep opened this issue Sep 6, 2021 · 4 comments · May be fixed by #777
Assignees
Projects

Comments

@hostep
Copy link
Contributor

hostep commented Sep 6, 2021

Preconditions (*)

  1. Seen on Magento OS 2.4.3
  2. Which comes with page-builder 1.7.0 (which isn't even published here on github? 😕 )
  3. I have nodejs v12 installed, but maybe it works with other versions as well

Steps to reproduce (*)

  1. Install Magento OS 2.4.3 and make sure page-builder is also installed
  2. Run npm install less@3.13.1
  3. Run rm -R var/view_preprocessed/* pub/static/*; bin/magento setup:static-content:deploy -f --theme=Magento/backend en_US
  4. Run node_modules/.bin/lessc -l var/view_preprocessed/pub/static/adminhtml/Magento/backend/en_US/css/styles.less

Expected result (*)

  1. The less linting should give no errors or warnings

Actual result (*)

  1. Linting the less file generates the following warnings:
extend ' .abs-reset-list' has no matches
extend ' .abs-add-box-sizing' has no matches
extend ' .abs-actions-addto' has no matches
extend ' .abs-product-link' has no matches
extend ' .abs-reset-image-wrapper' has no matches

Discussion

The issues come from the page-builder module, when I don't have page builder installed with Magento 2.4.3, I don't get linting errors.

We've seen similar problems before in core Magento in the following tickets/PR's. Maybe reading those can help you figure out what exactly is wrong and how to fix these problems:

It would be great if the less code from this module has no linting errors.
Would also be super nice if Magento's static tests could include a check for this, so it doesn't get introduced in any other new modules that are created in the future

@m2-assistant
Copy link

m2-assistant bot commented Sep 6, 2021

Hi @hostep. Thank you for your report.
To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this


@hostep
Copy link
Contributor Author

hostep commented Sep 6, 2021

One example about the .abs-reset-list warning

You are trying to use the .abs-reset-list class in your adminhtml less:

However, the definition of this class only exists on the frontend:

So this means that your code won't have an effect and should either be removed. Or - if you expect it to have an effect - to rework it so that it works as it's supposed to work.

I didn't investigate the other cases, they might be similar ...
Hope this helps 🙂

hostep added a commit to hostep/magento2-page-builder that referenced this issue Sep 6, 2021
@m2-community-project m2-community-project bot moved this from Ready for Grooming to Pull Request In Progress in Backlog Sep 6, 2021
hostep added a commit to hostep/magento2-page-builder that referenced this issue Jun 23, 2022
hostep added a commit to hostep/magento2-page-builder that referenced this issue Aug 24, 2022
hostep added a commit to hostep/magento2-page-builder that referenced this issue Feb 15, 2023
@m2-community-project m2-community-project bot moved this from Pull Request In Progress to Done in Backlog Mar 13, 2024
@hostep
Copy link
Contributor Author

hostep commented Mar 13, 2024

Finally 😅

@engcom-Charlie or @engcom-Hotel: any idea why #777 isn't marked as merged?

@hostep
Copy link
Contributor Author

hostep commented Mar 15, 2024

It's because commit 7d81e5c wasn't merged as well, can you guys please be a bit more careful when merging, so you don't miss stuff?

Can somebody fix this please? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Backlog
  
Done
1 participant