Skip to content

Make sure 'last' class is set on top menu #22071

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 31, 2019
Merged

Make sure 'last' class is set on top menu #22071

merged 4 commits into from
May 31, 2019

Conversation

arnoudhgz
Copy link
Contributor

Original PR: #13323 (was accidentally closed)

Description

The counter in the foreach loop was not being calculated correctly. This resolved in the 'last' css class not being set on the output when one or more parent items are not active.

With this PR the items will be filtered first before they are being processed for the html output.

Fixed Issues (if relevant)

  1. Topmenu 'last' class not being set if the a parent is inactive #13266: Topmenu 'last' class not being set if the a parent is inactive

Manual testing scenarios

  1. Do a clean install of Magento 2.2 with sample data
  2. Set a level 1 category (e.g. Gear or Gift Cards) inactive
  3. Refresh the appropriate cache
  4. Inspect the menu and see there is a class last on the last active top menu item

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)

@arnoudhgz
Make sure 'last' class is set on top menu
151a48e 
Previously the 'last' class was not set on the last visible top menu
item when one of the top level categories was disabled.

Also improved:
+ Split up the _getHtml() method to removed the @SuppressWarnings
+ Removed unneeded variables (moved the content of the variables to the
place where they are actually are being used).
+ Removed unused $itemPosition variable in _getHtml method
+ Optimized imports
@m2-assistant
Copy link

m2-assistant bot commented Mar 31, 2019

Hi @arnoudhgz. 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

@magento-engcom-team magento-engcom-team added Component: Theme Release Line: 2.3 Partner: MediaCT Pull Request is created by partner MediaCT partners-contribution Pull Request is created by Magento Partner labels Mar 31, 2019
@rogyar
Copy link
Contributor

rogyar commented Mar 31, 2019

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @rogyar. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @rogyar, here is your new Magento instance.
Admin access: https://pr-22071.instances.magento-community.engineering/admin
Login: admin Password: 123123q

@rogyar rogyar self-assigned this Mar 31, 2019
@magento-engcom-team
Copy link
Contributor

Hi @rogyar, thank you for the review.
ENGCOM-4623 has been created to process this Pull Request

@soleksii soleksii self-assigned this Apr 5, 2019
@soleksii
Copy link

soleksii commented Apr 5, 2019

✔️ QA Passed

Before:

before

After:

after

@p-bystritsky p-bystritsky self-assigned this Apr 16, 2019
@p-bystritsky p-bystritsky force-pushed the bugfix/13266-fix-processing-topmenu-items branch from d0b277d to 6a986b9 Compare May 15, 2019 09:17
@p-bystritsky p-bystritsky force-pushed the bugfix/13266-fix-processing-topmenu-items branch from 6a986b9 to cbedc60 Compare May 15, 2019 11:52
@p-bystritsky p-bystritsky force-pushed the bugfix/13266-fix-processing-topmenu-items branch from cbedc60 to 32caa04 Compare May 15, 2019 13:10
@p-bystritsky p-bystritsky force-pushed the bugfix/13266-fix-processing-topmenu-items branch from 32caa04 to 2a7f5b5 Compare May 15, 2019 14:10
@magento-engcom-team magento-engcom-team mentioned this pull request May 31, 2019
Closed
@magento-engcom-team magento-engcom-team merged commit 2a7f5b5 into magento:2.3-develop May 31, 2019
@m2-assistant
Copy link

m2-assistant bot commented May 31, 2019

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

magento-engcom-team pushed a commit to okorshenko/magento2 that referenced this pull request May 31, 2019
@p-bystritsky
ENGCOM-4623: Make sure 'last' class is set on top menu magento#22071
8bcfe4f
@magento-engcom-team magento-engcom-team added this to the Release: 2.3.3 milestone May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rogyar rogyar rogyar approved these changes

Assignees

@rogyar rogyar

@p-bystritsky p-bystritsky

@soleksii soleksii

Labels
Award: bug fix Component: Theme Partner: MediaCT Pull Request is created by partner MediaCT partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.3
Projects
None yet
Milestone
Release: 2.3.3
Development

Successfully merging this pull request may close these issues.
6 participants
@arnoudhgz @rogyar @magento-engcom-team @soleksii @sivaschenko @p-bystritsky