Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Hide menu from navigation if disabled from Magento admin #1022

Merged
merged 9 commits into from
Mar 25, 2019

Conversation

vishal-7037
Copy link
Contributor

@vishal-7037 vishal-7037 commented Mar 14, 2019

Category hide from navigation menu, if "Include in Menu" attribute set to false from Magento admin.

Description

Added field in graphql schema and added condition to hide menu from navigation.

Related Issue

Closes #1021

Motivation and Context

Verification

Screenshots (if appropriate):

Magento Configuration:
2019_03_14_Selection_001

Before Fixed:
2019_03_14_Selection_002

After Fixed:
2019_03_14_Selection_004

Proposed Labels for Change Type/Package

BUG

venia-concept

Checklist:

  • I have read the CONTRIBUTING document.
  • I have linked an issue to this PR.
  • I have indicated the change type and relevant package(s).
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All CI checks are green (linting, build/deploy, etc).
  • At least one core contributor has approved this PR.

@magento-cicd2
Copy link

magento-cicd2 commented Mar 14, 2019

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Mar 14, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@coveralls
Copy link

coveralls commented Mar 14, 2019

Coverage Status

Coverage increased (+0.04%) to 76.672% when pulling efba0d0 on vishal-7037:bug/navigation-menu into 09cea02 on magento-research:develop.

Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Please add a test for the case where include_in_menu is falsy.

@vishal-7037
Copy link
Contributor Author

I've added a test for the case where include_in_menu is falsy.

@vishal-7037
Copy link
Contributor Author

@sirugh I've added a test case where include_in_menu is falsy. Is anything remain from my side or your side?

sirugh
sirugh previously approved these changes Mar 18, 2019
Copy link
Contributor

@sirugh sirugh 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 adding the test case!

@sirugh sirugh added the bug Something isn't working label Mar 21, 2019
Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

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

I fixed the unit tests and was about to merge, but found a minor coercion problem that was worth sending back your way. Should be good to go after that change.

@tjwiebell tjwiebell merged commit 822591e into magento:develop Mar 25, 2019
@sirugh sirugh added the version: Minor This changeset includes functionality added in a backwards compatible manner. label May 24, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Category displayed in navigation menu, if "Include in Menu" attribute set to false from Magento admin.
6 participants