Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

Add module for a cleaner navigation walker #73

Merged
merged 1 commit into from
Apr 16, 2015
Merged

Conversation

retlehs
Copy link
Member

@retlehs retlehs commented Apr 14, 2015

this is the same walker from the sage theme except for:

@retlehs retlehs force-pushed the cleaner-nav-walker branch from e9a7182 to 1bd1c73 Compare April 14, 2015 18:56
@retlehs
Copy link
Member Author

retlehs commented Apr 14, 2015

@vinkla thanks, but will tackle those outside of this PR.

@@ -50,6 +50,8 @@ public function set($options) {
}
}

require_once(plugin_dir_path(__FILE__) . 'lib/utils.php');
Copy link
Member

Choose a reason for hiding this comment

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

plugin_dir_path is just a wrapper for dirname(__FILE__)

Could use require_once __DIR__ . '/lib/utils.php;

@retlehs retlehs force-pushed the cleaner-nav-walker branch from 1bd1c73 to 9fe3208 Compare April 14, 2015 19:51
@vinkla
Copy link
Contributor

vinkla commented Apr 15, 2015

@retlehs okay, great. Let me know if I can do anything to help.

Out of curiosity, what are the standards set for this package?

@vinkla
Copy link
Contributor

vinkla commented Apr 15, 2015

What about naming the directory src instead of lib? src is much more common than lib.

Since nav-walker.php contains a class I think it would be good to show that in the file name. This is also common practice. I think we should name it NavWalker.php instead.

@swalkinshaw
Copy link
Member

@vinkla
Copy link
Contributor

vinkla commented Apr 15, 2015

@swalkinshaw Yes, exactly what I was looking for. Thanks!

QWp6t added a commit that referenced this pull request Apr 16, 2015
Add utils and cleaner nav walker module
@QWp6t QWp6t merged commit 28d4f34 into master Apr 16, 2015
@QWp6t QWp6t deleted the cleaner-nav-walker branch April 16, 2015 23:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants