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

Removing incorrect less selector '.abs-cleafix', it has a typo + it i… #22184

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Apr 6, 2019

…s already used correctly in a different less file (_data-grid-header.less).

Description (*)

This was discovered by using a different less compiler then what Magento ships with.

There is a typo in the _temp.less file where it says .abs-cleafix instead of .abs-clearfix
After some further research, it looks like the same extends is already correctly added in a different place to the same selector .admin__data-grid-header-row over here, so we don't need to fix the incorrect selector, but can just remove it.

More details:

  • .abs_clearfix references the .lib-clearfix() mixin.
  • The .lib-clearfix() mixin is defined over here.
  • So this means that the .admin__data-grid-header-row selector's ::before & ::after pseudo-elements get the properties as defined by the .lib-clearfix() mixin. And this stays the case after removing the line in this PR since it's also being applied over here.

Fixed Issues (if relevant)

None that I could find.

Manual testing scenarios (*)

  1. Having nodejs v8 installed
  2. Setting up vanilla Magento installation using composer
  3. Running npm install less@2.7.3
  4. Open Magento's backend in the browser, so static files for the admin theme are being generated
  5. Re-compile less, now using the less.js compiler: node_modules/.bin/lessc --no-color var/view_preprocessed/pub/static/adminhtml/Magento/backend/en_US/css/styles.less | head
  6. Notice that without this fix the first line says: extend ' .abs-cleafix' has no matches, with this fix that error is gone.
  7. In the backend, go to the product grid, look in your inspector and search for elements having the admin__data-grid-header-row class, and look at their ::before & ::after pseudo-elements.
  8. With and without this fix, it is expected to see that those pseudo-elements have the css properties:
content: '';
display: table;

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 on Travis CI are green)

…s already used correctly in a different less file (_data-grid-header.less).
@m2-assistant
Copy link

m2-assistant bot commented Apr 6, 2019

Hi @hostep. 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

Copy link
Contributor

@Karlasa Karlasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clean up :)

@VasylShvorak
Copy link
Contributor

✔️ QA passed

@m2-assistant
Copy link

m2-assistant bot commented Apr 15, 2019

Hi @hostep, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants