This module solves the problem of creting a mobile version for any menu on drupal. It has a way to let us specify any menu to be turned in to a mobile friendly navigation menu and also has a configuration page that let us specify various types, effects and features for the mobile version of menus.
The only project that i found that does somthing simmilar is the Mobile Friendly Navigation Toolbar, taken from the spark project. This module creates an admin menu mobile friendly version, but that module is intended to work only in the administration menu navigation and has only one type of navigation.
Mobile Navigation module tries to give users endless possibilities to configure the mobile version of any menu on the website, having the chance to choose any of the diferent kinds of mobile menus out there.

Pareview.sh online: http://pareview.sh/pareview/httpgitdrupalorgsandboxx7ian1996698git

Check the Demo http://mndemo.geekfinger.com

The project is currently in a sandbox here: http://drupal.org/sandbox/x7ian/1996698
To download use git: http://drupal.org/project/1996698/git-instructions

Other Projects Reviews:

Subviews module https://drupal.org/node/2004838
Blocks Mass Cache https://drupal.org/node/2035499#comment-7630053
Language Switcher Settings https://drupal.org/node/2035427#comment-7641155

Comments

brice_gato’s picture

brice_gato commented

Hello @x7ian,
Please add this line into your mobile_navigation.info file :
configure = admin/config/user-interface/mobile_navigation

brice_gato’s picture

brice_gato commented
Status: Needs review » Needs work
x7ian’s picture

x7ian commented

done, added!
can you please help me a little more on how i can review other projects.
I tested a module from the Project Applications list, and raised an issue about a bug i found but i dunno if there is something else i should do to review the project and gain points as a reviewer.
Thank you brice!

brice_gato’s picture

brice_gato commented

Hello,
Here is a link where you can find out how to get on the high priority review list :
http://drupal.org/node/1975228

x7ian’s picture

x7ian commented

Thank you Brice,
I have a question,
Ive made a some more commits and today i did a "git log" command,
and some of the commits that showing me are made my me "x7ian"
but some are made by "geekfinger". Both profiles are the same, me, but i dont get why some commits are being made by x7ian and some by geekfinger.
Geekfinger is an account i had to make cause i lost my access to de email on file for x7ian for some time.
But then i got x7ian back and kept on adentifiying my self with x7ian.
I dont know why commits are being registered by geekfinger, i dont remember loging in with that user or using it to identify on git.
So ive changed user and email using "git config user.name" and "git config user.email", to "x7ian" and my email "yo@cgalicia.me".
Then i did other commit, and now it was registered by x7ian, but it has no link to my profile, i think is not identifying my profile.
You can check the commits and youll see that some comits are being made by x7ian, other by geekfinger and the last by x7ian but has no link to my profile.
What should i do to fix this? :-s
Thank you for your help!

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

hi christian, i really like this module and though my time is verrry limited i might help review or code.

you can fix the git commits by "rewriting history" through "interactive rebasing" (google that).
feel free to pm me for further git support, i won't hang out in the issues often these days ;-)

x7ian’s picture

x7ian commented

Thank you axel.rutz for your help,
Im fixing the commits with that info,
and thank very much you for your comments and issues about the module!
;-)

x7ian’s picture

x7ian commented
Issue summary: View changes

Original post

x7ian’s picture

x7ian commented
Status: Needs work » Needs review

Ready for moving to Needs Review status.

PA robot’s picture

PA robot commented

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

jorgegc’s picture

jorgegc commented
Status: Needs review » Needs work

Hi @x7ian,

I have had a quick look at your module and here is what I found:

1. CANGELOG.txt
- Shouldn't it be called CHANGELOG.txt? :-)
- Add a blank newline at the end of the file.

2. In mobile_navigation.admin.inc
- Lines 58, 70 and 81: Not a big deal but I would create helper functions that return those arrays just to keep the code easier to maintain.
- All the t function calls: Sometimes you use single quotes, sometimes double quotes. I am not sure if there is a convention for this but I would pick one style and stick with it.
- Add a blank newline at the end of the file.

3. In mobile_navigation.css
- Check how you are doing comments and blank lines against what is described here https://drupal.org/node/1887862
- Add a blank newline at the end of the file.

4. In mobile_navigation.info
- Add a blank newline at the end of the file.

5. In mobile_navigation.js
- Lines 14 and 27: You should be using the context to find the element that you are looking for otherwise you could potentially be running the same code twice unnecessarily. ie: $(Drupal.settings.mobile_navigation.menuSelector, context)
- Line 11: Check out how we do comments in JS in Drupal here https://drupal.org/node/172169
- Add a blank newline at the end of the file.

6. In mobile_navigation.module
- Lines 34 to 56: Do you really want to add your JS and CSS to every single page? Even admin pages? Something to think about.
- Lines 62: I don't think there is a rule for this but I wouldn't use underscores in my paths. I would always use a hifen to keep it nice and clean.
- Add a blank newline at the end of the file.

I think that is all, your code is looking pretty good overall!

jorgegc’s picture

jorgegc commented
Issue summary: View changes

Added other project review

x7ian’s picture

x7ian commented

Thank you for your review.
There is a blank line at the end of all the files. If there werent the PAReview.sh would be pointing out that as errors.

1. Changed to CHANGELOG.txt, ;-)
Blank line is there at the end, as well as on all other files. PAReview.sh ask me to add it quite a few commits ago.

2. Took your suggestion to use functions to get the options on 58 and 81, to make it easier to maintain in case new options arise in the future, which there will surely be. Not in 70 since for that particular selection there will be no new options added in the future.
Blank line is there at the end, as well as on all other files. PAReview.sh ask me to add it quite a few commits ago.

3. Ive read the post at https://drupal.org/node/1887862. I'm not sure what you're are pointing out here.
Blank line is there at the end, as well as on all other files. PAReview.sh ask me to add it quite a few commits ago.

4. Blank line is there at the end, as well as on all other files. PAReview.sh ask me to add it quite a few commits ago.

5. Added ", context" to selectors.
Blank line is there at the end, as well as on all other files. PAReview.sh ask me to add it quite a few commits ago.

6. What if the user whats to use mobile navigation on administration pages? This will require a field added that specifies for what theme will the plugin be needed. For now i added a condition so that the mobile navigation will work only at the current theme, not the admin. In a future we will have to add the option for the user to specify if they want to use it at the admin theme.
OK, i changed "_" to "-" to make it standard.
Blank line is there at the end, just as in the other files.

fixes commited here: http://drupalcode.org/sandbox/x7ian/1996698.git/commit/8e9c477

x7ian’s picture

x7ian commented
Status: Needs work » Needs review

Setting it to needs review, after some more fixes and testing.

x7ian’s picture

x7ian commented
Issue summary: View changes

Updated issue summary.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Postponed (maintainer needs more info)

Don't forget to add the "PAReview: review bonus" tag as indicated in http://drupal.org/node/1975228 , otherwise you won't show up on my high priority list.

https://drupal.org/project/responsive_menus

This sounds like a feature that should live in the existing responsive_menus project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the responsive_menus issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

x7ian’s picture

x7ian commented
Issue tags: +PAreview: review bonus

Thank you @klausi,
Here is the issue i created at the responsive_menus module issue queue.

https://drupal.org/node/2045063

Ill be posting here any progress in this possible module duplication conflict.

x7ian’s picture

x7ian commented
Issue summary: View changes

added other project reviews

x7ian’s picture

x7ian commented

Hello @klausi,
Ive tested and analized the option of merging Mobile Navigation with Responsive Menus or making it a dependent module, and ended up with the conclusion that ill better go for the independent module path. I did testing and thought of this a lot and ended up with this conclusion.
The reason for this is that the Responsive Menus module would become only a wrapper for Mobile Navigation to override everything that it does. Responsive Menus currently has a JS plugin that is very basic and in a very early stage of development, compared to Mobile Navigation which has been improved a lot with all the issues that have been solved during the last two months.
I love the idea of contributing instead of competing but in this case i don't see a practical advantage on making this module dependent on Responsive Menus. If we make Mobile Navigation to become an add on module for Responsive Menus it will be resulting in Mobile Navigation to be dependent on other module with no real advantage on this. There is no functionality at all that Mobile Navigation can take advantage of by being dependent on Responsive Menus, on the other hand i see more work for users that will have to install two modules just for Mobile Navigation to use a Hook that is declared in Responsive Menus. I belive this can also be a load for Drupal since it will be usign more hooks.
Mobile Navigation goal is to be a complete solution to implement all of the mobile menus ive seen in the web, something that it currently does pretty well just the way it is.
I don't think there is a problem on Mobile Navigation being an independent module, there are other cases of modules that does something similar (like there is Superfish, Nice Menus, Mega Menu, and others that implements menus for desktop).
For the last two months many people has tried it, like it and help improve it and it has become a very complete module that is being used in production by quite a few sites already. I believe it is pretty ready for deployment.So i belive Mobile Navigation should become an independent module.

isimgt’s picture

isimgt commented
Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

There are some sites using this module in production and some people has helped testing it. I think it is quite reviewed and tested by the community.

isimgt’s picture

isimgt commented
Issue summary: View changes

Added the demo page link.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Status: Reviewed & tested by the community » Needs work

Thanks for addressing the duplication issue - from reading the linked post it seems there is a possibility for collaboration, but the modules are significantly different.

Some minor points: use single quotes wherever possible, they're faster and that's our coding standard. You have a typo in the module file, "acordion" should be "accordion". Setting to needs work for the 3rd party code, but otherwise this module looks good.

PAReview: 3rd party code
mobile_navigation v1.0 - Mobile Navigation appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms.

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

----
Top Shelf Modules - Crafted, Curated, Contributed.

x7ian’s picture

x7ian commented
Status: Needs work » Needs review

Thank you @kscheirer,

use single quotes wherever possible, they're faster and that's our coding standard.

Changed Double Quotes to single Quotes wherever possible, thank you for the correction.

you have a typo in the module file, "acordion" should be "accordion"

Fixed. Changed all acordion to Accordion in the files where it is used.

3rd party code

Mobile navigation is not third party code. This library was born with this project as a Drupal module. There is no js project apart from the code built with the module, it has been reviewed and tested on the issues of the sandbox. That's the main argument for this module to be independent from Responsive Menus which is a module that serves as wrapper for third party libraries.
In a future this may change since it has become a very complete and robust Js solution, the idea has arouse for making it a third party library, however this will be in a future version, for now we are focused on adding some new features making it as stable as possible as a drupal module, in a future it may become a third party JS library.
Thank you for reviewing.

flebrenn’s picture

flebrenn commented
Status: Needs review » Needs work

Hi,

Automatic code review
HIDESITES/ALL/MODULES/CUSTOM/MOBILE_NAVIGATION/JS/MOBILE_MENU.JS
mobile_menu.js: severity: normalreview: comment_docblock_fileFile: @file block missing (Drupal Docs) [comment_docblock_file]

HIDESITES/ALL/MODULES/CUSTOM/MOBILE_NAVIGATION/MOBILE_NAVIGATION.JS
mobile_navigation.js: severity: normalreview: comment_docblock_fileFile: @file block missing (Drupal Docs) [comment_docblock_file]

Manual code review
Please fix parse_error (admin/config/user-interface/mobile-navigation).

Bye,

x7ian’s picture

x7ian commented

Thank you @flebrenn,

Manual code review
Please fix parse_error (admin/config/user-interface/mobile-navigation).

Oups, Sorry about that error i left on the module after the lar commit,
is now fixed.

Automatic code review
HIDESITES/ALL/MODULES/CUSTOM/MOBILE_NAVIGATION/JS/MOBILE_MENU.JS
mobile_menu.js: severity: normalreview: comment_docblock_fileFile: @file block missing (Drupal Docs) [comment_docblock_file]

I couldnt get rid of this errors showed by code sniffer.
Im placing the @file block there as showed here https://drupal.org/node/1354#files
However Code sniffer is not recognizing it.
I found this post about this problem, https://drupal.org/node/1834598
Seems like it could be a problem on code sniffer.

x7ian’s picture

x7ian commented
Status: Needs work » Needs review

It looks like Code Sniffer has problems recognizing the @file block on .js files.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: Unassigned » patrickd
Status: Needs review » Reviewed & tested by the community
Issue tags: -PAreview: review bonus

manual review:

  1. Please describe the differences to similar modules such as responsive_navigation and responsive_menus on the project page, so that users can make an educated decision which module to use.
  2. mobile_menu.js: 1000 lines of javascript - a bit heavy for a responsive menu?
  3. mobile_navigation_init(): I think you should use hook_page_build() instead, since you won't need the JS on requests that don't render a page.
  4. "variable_get('mobile_navigation_breakpoint', MOBILE_NAVIGATION_BREAKPOINT)": all variables defined by your module need to be removed in hook_uninstall().

But that are not blockers, otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to patrickd as he might have time to take a final look at this.

patrickd’s picture

patrickd commented
Status: Reviewed & tested by the community » Fixed

Beside the issues pointed out by klausi, please try not to break lines in strings within the t() function. This makes translation of these strings a bit ugly.

Thanks for your contribution!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

x7ian’s picture

x7ian commented

Thank you,
@klausi

1. The information explaining differences to other similar projects is added on the project page.
2. The reason for the file being so heavy is that the library has a lot of functionality.
im working on a way to split it into various files and load the js on demand, acording to the functionalities that are configured.
3. Stuff moved from hook_init() to hook_page_build(). Thank you for the observation.
4. Added the hook_uninstall().

@patrickd

Ill definitely be helping more with reviews and contributing in a future.
Ill also work on making this module grow more.
Thank you for helping me with reviewing and the teachings.
Ive learned a lot with this project.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

added link to pareview.sh online