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

include setting on item #30

Merged
merged 3 commits into from Apr 23, 2018
Merged

include setting on item #30

merged 3 commits into from Apr 23, 2018

Conversation

tg666
Copy link
Contributor

@tg666 tg666 commented Apr 9, 2018

  • added setting include to item's configuration (value can be string or array of strings)
  • added method IMenuItem::setInclude()
  • added test

- added setting `include` to item's configuration (value can be string or array of strings)
- added method `IMenuItem::setInclude()`
- added test
@davidkudera
Copy link
Contributor

Thank you, good work 👍

Do you think you could also add some note into the readme file please?

@tg666
Copy link
Contributor Author

tg666 commented Apr 10, 2018

Of course, here it is 🙂

@tg666
Copy link
Contributor Author

tg666 commented Apr 19, 2018

@davidkudera can you merge it please? 🙂

@davidkudera
Copy link
Contributor

@tg666 sure, I was just waiting for you with the review. You should see one a bit above here. Can you please fix it? I'll merge it immediately afterwards.

@@ -4,6 +4,7 @@

namespace Carrooi\Menu\Loaders;

use Nette\Utils;
Copy link

Choose a reason for hiding this comment

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

Sort it alphabetically please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@davidkudera davidkudera merged commit 33d453c into contributte:master Apr 23, 2018
@davidkudera
Copy link
Contributor

Thank you and good job 👍

also sorry for the problems....

@tg666
Copy link
Contributor Author

tg666 commented Apr 23, 2018

No problem, thank you too 👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants