• Global community
    • Language:
      • Deutsch
      • English
      • Español
      • Français
      • Português
  • 日本語コミュニティ
    Dedicated community for Japanese speakers
  • 한국 커뮤니티
    Dedicated community for Korean speakers
Exit
0

[JS] Menu Added via Scripting Moves

Community Expert ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

I've been able to sucessfully add items to the InDesign menu thanks in part to Marijan Tompa's (tomaxxi) blog post http://indisnip.wordpress.com/2010/08/08/create-customized-menu/

My test code (below) creates a new menu, and sucessfully adds two menu items plus a submenu. The submenu is causing me problems. When you first launch InDesign it's created in the proper place (in the middle of the menu). But when you relaunch InDesign, the submenu moves to the top of the menu and never goes back to it's proper position farther down in the menu where it was the first time InDesign was launched.

I've searched high and low in this forum, the web and InDesign's documentation and can't figure out how to keep it from moving (I want the menu to be farther down the menu, not at the top). I hope one of you kind souls will help me to control the position of the submenu (and have it stay there across launches).

Thanks in advance!

Dan

Here's the code I'm working with. This is saved as testMenu.jsx in the Scripts > startup scripts folder.

P.S. I'm testing this in CS5.5 currently, but ideally this solution should work in CS3 and later.

#targetengine "myTestMenu"

var myFolder = Folder(app.activeScript.path);

myFolder = myFolder.parent + '/Scripts Panel/';

var menuItem1Handler = function( /*onInvoke*/ ){

  app.doScript(File(myFolder + 'MyTest1.jsx'));

};

var menuItem2Handler = function( /*onInvoke*/ ){

  app.doScript(File(myFolder + 'MyTest2.jsx'));

};

menuInstaller()

function menuInstaller() {

  var menuItem1T = "My Menu Item 1",

       menuItem2T = "My Menu Item 2",

       menuT = "MyTestMenu",

             subT = "Sub Menu",

       subs = app.menus.item("$ID/Main").submenus, sma, mnu;

  var refItem = app.menus.item("$ID/Main").submenus.item("$ID/&Layout");

  subMenu1 = app.scriptMenuActions.item(menuItem1T);

  if( subMenu1 == null ) {

          subMenu2 = app.scriptMenuActions.add(menuItem1T);

  }

  subMenu2 = app.scriptMenuActions.item(menuItem2T);

  if( subMenu2 == null ) {

          subMenu2 = app.scriptMenuActions.add(menuItem2T);

  }

  subMenu2.eventListeners.add("onInvoke", menuItem2Handler);

  mnu = subs.item(menuT);

  if( mnu == null ) {

            mnu = subs.add(menuT, LocationOptions.after, refItem);

  }

  mnu.menuItems.add(subMenu1);

  mnu.menuSeparators.add();

  subsSubs = app.menus.item( '$ID/Main' ).submenus.item( menuT ).submenus;

  mnuSubMenu = subsSubs.item( subT );

  if( mnuSubMenu == null ) {

          mnuSubMenu = subsSubs.add( subT);

  }

  mnu.menuItems.add(subMenu2);

};


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor
TOPICS
Scripting

Views

19.7K

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines

correct answers 1 Correct answer

Community Expert , Dec 17, 2011 Dec 17, 2011

Thank you all so much for your replies, and thank you Harbs for making it finally work!

Just to make sure this can serve as a resource for others in the future, below is a final (working) menu based on Harb's code. I also added 2 submenu items just to be complete.

Thanks again everyone. I'm so glad you could help get this working!

Dan

#targetengine "HarbsTestMenu"

menuInstaller();

function menuInstaller() {

    // SET THE FILES THAT ARE TRIGGERED BY MENU ITEMS

    menuItem1Handler = function( /*onInvo

...

Votes

Translate

Translate
LEGEND ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

Dan, I haven't gone and tested this in your example, so this is speculative but:

  subMenu1 = app.scriptMenuActions.item(menuItem1T);
  if( subMenu1 == null ) {
          subMenu2 = app.scriptMenuActions.add(menuItem1T);
  }

This isn't really advisable. In CS5 and later, if you don't check .isValid, you can have problems (I think).

But also, in JavaScript, in general, you should not write

if (varname == null)

because it is usually sufficient to simply write

if (varname)

or

if (!varname)

and you should also never use the == operator when you can use the === operator, because it coerces to truthiness. So you should rather instead use

if (varname===null)

if you truly needed to check for null as opposed to undefined or zero.

Perhaps more significnatly, shouldn't you be setting submenu1 inside that if, rather than submenu2?

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

  1. .isValid doesn't seem to fix the issue of the submenu moving. It also doesn't work back in CS3. I'd like one set of code (if possible) that will work in CS3 and later. Although if I need two sets of code I can deal with that, as long as I know what would make it work in each version. The submenu is still moving and that's the real issue.
  2. Switching from == to === doesn't fix the issue of the submenu moving (although the == was working even if the error detection isn't as thruthful).
  3. Thanks for the eagle eye on the typo of submenu1 instead of 2, but that also doesn't fix it.

Taking into account your suggestions though, here's a slight update on the code, but the submenu still moves on relaunch. Otherwise it works, it's just that the submenu moves. Any more thoughts?

#targetengine "myTestMenu"

var myFolder = Folder(app.activeScript.path);

myFolder = myFolder.parent + '/Scripts Panel/';

var menuItem1Handler = function( /*onInvoke*/ ){

  app.doScript(File(myFolder + 'MyTest1.jsx'));

};

var menuItem2Handler = function( /*onInvoke*/ ){

  app.doScript(File(myFolder + 'MyTest2.jsx'));

};

menuInstaller()

function menuInstaller() {

  var menuItem1T = "My Menu Item 1",

       menuItem2T = "My Menu Item 2",

       menuT = "MyTestMenu",

             subT = "Sub Menu",

       subs = app.menus.item("$ID/Main").submenus, sma, mnu;

  var refItem = app.menus.item("$ID/Main").submenus.item("$ID/&Layout");

  subMenu1 = app.scriptMenuActions.item(menuItem1T);

  if( subMenu1 === null ) {

          subMenu1 = app.scriptMenuActions.add(menuItem1T);

  }

  subMenu2 = app.scriptMenuActions.item(menuItem2T);

  if( subMenu2 === null) {

          subMenu2 = app.scriptMenuActions.add(menuItem2T);

  }

  subMenu2.eventListeners.add("onInvoke", menuItem2Handler);

  mnu = subs.item(menuT);

  if( mnu === null ) {

            mnu = subs.add(menuT, LocationOptions.after, refItem);

  }

  mnu.menuItems.add(subMenu1);

  mnu.menuSeparators.add();

  subsSubs = app.menus.item( '$ID/Main' ).submenus.item( menuT ).submenus;

  mnuSubMenu = subsSubs.item( subT );

  if( mnuSubMenu === null) {

          mnuSubMenu = subsSubs.add( subT);

  }

  mnu.menuItems.add(subMenu2);

};


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

Here's another variation without the ===null. But the same thing happens on relaunch. The submenu moves up.

I've read over http://www.indiscripts.com/post/2010/02/how-to-create-your-own-indesign-menus but I don't see any advice there either to help on this issue. There has to be some small thing I'm missing!

#targetengine "myTestMenu"

var myFolder = Folder(app.activeScript.path);

myFolder = myFolder.parent + '/Scripts Panel/';

var menuItem1Handler = function( /*onInvoke*/ ){

  app.doScript(File(myFolder + 'MyTest1.jsx'));

};

var menuItem2Handler = function( /*onInvoke*/ ){

  app.doScript(File(myFolder + 'MyTest2.jsx'));

};

menuInstaller()

function menuInstaller() {

  var menuItem1T = "My Menu Item 1",

       menuItem2T = "My Menu Item 2",

       menuT = "MyTestMenu",

             subT = "Sub Menu",

       subs = app.menus.item("$ID/Main").submenus, sma, mnu;

  var refItem = app.menus.item("$ID/Main").submenus.item("$ID/&Layout");

  subMenu1 = app.scriptMenuActions.item(menuItem1T);

  if( subMenu1 ) {

          subMenu1 = app.scriptMenuActions.add(menuItem1T);

  }

  subMenu2 = app.scriptMenuActions.item(menuItem2T);

  if( subMenu2) {

          subMenu2 = app.scriptMenuActions.add(menuItem2T);

  }

  subMenu2.eventListeners.add("onInvoke", menuItem2Handler);

  mnu = subs.item(menuT);

  if( mnu ) {

            mnu = subs.add(menuT, LocationOptions.after, refItem);

  }

  mnu.menuItems.add(subMenu1);

  mnu.menuSeparators.add();

  subsSubs = app.menus.item( '$ID/Main' ).submenus.item( menuT ).submenus;

  mnuSubMenu = subsSubs.item( subT );

  if( mnuSubMenu ) {

          mnuSubMenu = subsSubs.add( subT);

  }

  mnu.menuItems.add(subMenu2);

};


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Beginner ,
Oct 20, 2016 Oct 20, 2016

Copy link to clipboard

Copied

how to add menu items in looping and how to invoke these menus. please help us...

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

If I run your script twice in a single session, I get errors.

It also looks like it's not designed to work from the ESTK (depending on app.activeScript.path), which suggest you're not debugging this very efficiently...

This error is undoubtedly due to your script not accounting for menu items that may already exist, and that's the case across a restart of InDesign -- menus persist.

You might want to add some $.writeln()s that run as it is looking for existing menus and reference points and whatnot.

Good luck!

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

OK, so the file reference didn't work from ESTK. Here's a new version that does things a bit differently. Running this from ESTK I no longer get any errors.

As far as I can tell, the problems are related to testing for existing menu items, because they are being created again. But I still don't know how to properly test for them and only add them if needed. I feel like I'm so close, but yet so far away! Any help is greatly appreciated.

#targetengine "myTestMenu"

installMenu();

function installMenu() {

 

          // SET THE FILES THAT ARE TRIGGERED BY MENU ITEMS

          menuActionOneHandler = function( /*onInvoke*/ ){

                     app.doScript( File(myFolder + 'File-One.jsx') );

          };

          menuActionTwoHandler = function( /*onInvoke*/ ){

                    app.doScript( File(myFolder + 'File-Two.jsx') );

          };

          // SET THE TEXT TO APPEAR IN MENUS

          var menuTextOne         = 'My Menu Item 1' ;

          var menuTextTwo                     = 'My Menu Item 2' ;

 

          var menuTitleMain           = 'My Test Menu' ;

          var menuTitleSubMenu          = 'Sub Menu' ;

 

          //CREATE MENU

          windowMenu = app.menus.item( '$ID/Main' ).submenus.item( '$ID/&Window' );

          myMainMenu = app.menus.item( '$ID/Main' ).submenus.item( menuTitleMain)

          if( !myMainMenu.isValid ) {

                    var myMainMenu = app.menus.item( '$ID/Main' ).submenus.add( menuTitleMain, LocationOptions.after , windowMenu)

          }

 

          // SET GENERAL VARIABLES

    try {

        myFolder = app.activeScript;

    }

    catch(e) {

        myFolder = File(e.fileName);  // we are running from the ESTK

    }

    myFolder = myFolder.parent.parent  + '/' ;  //this file is in the "startup scripts" subfolder

 

 

          //CREATE MENU ITEMS

          menuActionOne = myMainMenu.submenus.item( menuTextOne );

          if( menuActionOne == null ) {

                    menuActionOne = app.scriptMenuActions.add( menuTextOne );

          }

          menuActionOne.eventListeners.add( 'onInvoke', menuActionOneHandler );

          myMainMenu.menuItems.add( menuActionOne );

 

          myMainMenu.menuSeparators.add(); 

 

          //CREATE SUB MENU 

                              try {

                var mySubMenu = myMainMenu.submenus.item( menuTitleSubMenu );

                mySubMenu.title;

                              }

                              catch (e) {

                var mySubMenu = myMainMenu.submenus.add( menuTitleSubMenu );

                              }

          //END OF SUB MENU 

          menuActionTwo = myMainMenu.submenus.item( menuTextTwo );

          if( menuActionTwo == null ) {

                    menuActionTwo = app.scriptMenuActions.add( menuTextTwo );

          }

          menuActionTwo.eventListeners.add( 'onInvoke', menuActionTwoHandler );

          myMainMenu.menuItems.add( menuActionTwo );

    return true;

} // end of installMenu()


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

As far as I can tell, the problems are related to testing for existing menu items, because they are being created again. But I still don't know how to properly test for them and only add them if needed. I feel like I'm so close, but yet so far away! Any help is greatly appreciated.

So, you're saying that .submenus.item("whatever").isValid and menus.item("whatever").isValid do not correctly tell you whether the menus exist? They should!

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

Hi Dan,

Add the piece of code in before adding the menus to the main menu.

try {

                    app.menus.item("$ID/Main").submenus.item(menuTitleMain).remove();

                }catch(e){}

I Hope this helps to you..

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

Imagine:

Add the piece of code in before adding the menus to the main menu.

try { app.menus.item("$ID/Main").submenus.item(menuTitleMain).remove();

But that's wrong! It means you'll reset the menu item every time. Among other things, that means if your user assigns a KBSC, it will be lost.

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

is it possible to assaign a KBSC for our Created Menus?

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

Well, for menu items, yes. They appear in the Scripts area.

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Dec 16, 2011 Dec 16, 2011

Copy link to clipboard

Copied

Hey Dan it has few side effect with my code,  try John suggested.  Thanks John.

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 17, 2011 Dec 17, 2011

Copy link to clipboard

Copied

[I removed my nonsensical post. See my response below...]

Harbs

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 17, 2011 Dec 17, 2011

Copy link to clipboard

Copied

Note to self: Don't respond to the forum before your brain is engaged...

Ignore what I wrote above. It made no sense on a lot of levels...

The issue at hand is caused by the following:

Submenus do not go away until you remove them. When restart InDesign, the submenu still exists, so you are creating the meni tems below it.

You need to create the submneu first and specify LocationOptions. Something like this:

function menuInstaller() {

  var menuItem1T = "My Menu Item 1",

       menuItem2T = "My Menu Item 2",

       menuT = "MyTestMenu",

             subT = "Sub Menu",

       subs = app.menus.item("$ID/Main").submenus, sma, mnu;

  var refItem = app.menus.item("$ID/Main").submenus.item("$ID/&Layout");

  subMenu1 = app.scriptMenuActions.item(menuItem1T);

  if( subMenu1 == null ) {

          subMenu1 = app.scriptMenuActions.add(menuItem1T);

  }

  subMenu1.eventListeners.add("onInvoke", menuItem1Handler);

  subMenu2 = app.scriptMenuActions.item(menuItem2T);

  if( subMenu2 == null ) {

          subMenu2 = app.scriptMenuActions.add(menuItem2T);

  }

  subMenu2.eventListeners.add("onInvoke", menuItem2Handler);

  mnu = subs.item(menuT);

  if( mnu == null ) {

            mnu = subs.add(menuT, LocationOptions.after, refItem);

  }

  subsSubs = app.menus.item( '$ID/Main' ).submenus.item( menuT ).submenus;

  mnuSubMenu = subsSubs.item( subT );

  if( mnuSubMenu == null ) {

          mnuSubMenu = subsSubs.add( subT);

  }

  mnu.menuItems.add(subMenu1,LocationOptions.BEFORE,mnuSubMenu);

  mnu.menuSeparators.add(LocationOptions.BEFORE,mnuSubMenu);

  mnu.menuItems.add(subMenu2,LocationOptions.AFTER,mnuSubMenu);

};

Harbs

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Dec 17, 2011 Dec 17, 2011

Copy link to clipboard

Copied

Thank you all so much for your replies, and thank you Harbs for making it finally work!

Just to make sure this can serve as a resource for others in the future, below is a final (working) menu based on Harb's code. I also added 2 submenu items just to be complete.

Thanks again everyone. I'm so glad you could help get this working!

Dan

#targetengine "HarbsTestMenu"

menuInstaller();

function menuInstaller() {

    // SET THE FILES THAT ARE TRIGGERED BY MENU ITEMS

    menuItem1Handler = function( /*onInvoke*/ ){

        app.doScript( File(myFolder + 'File-One.jsx') );

    };

    menuItem2Handler = function( /*onInvoke*/ ){

        app.doScript( File(myFolder + 'File-Two.jsx') );

    };

    subMenuItem1Handler = function( /*onInvoke*/ ){

        app.doScript( File(myFolder + 'File-Three.jsx') );

    }; 

    subMenuItem2Handler = function( /*onInvoke*/ ){

        app.doScript( File(myFolder + 'File-Four.jsx') );

    }; 

 

    // SET GENERAL VARIABLES

    try {

        myFolder = app.activeScript;

    }

    catch(e) {

        myFolder = File(e.fileName);  // we are running from the ESTK

    }

    myFolder = myFolder.parent.parent  + '/' ;  //this file is in the "startup scripts" subfolder

 

    var menuItem1T = "My Menu Item 1",

       menuItem2T = "My Menu Item 2",

       subMenuItem1T = "My SubMenu Item 1",

       subMenuItem2T = "My SubMenu Item 2",

       menuT = "HarbsTestMenu",

       subT = "Sub Menu",

       subs = app.menus.item("$ID/Main").submenus, mnu;

    var refItem = app.menus.item("$ID/Main").submenus.item("$ID/&Layout");

    // CREATE MENU ITEMS

    subMenu1 = app.scriptMenuActions.item(menuItem1T);

    if( subMenu1 == null ) {

        subMenu1 = app.scriptMenuActions.add(menuItem1T);

    }

    subMenu1.eventListeners.add("onInvoke", menuItem1Handler);

    subMenu2 = app.scriptMenuActions.item(menuItem2T);

    if( subMenu2 == null ) {

        subMenu2 = app.scriptMenuActions.add(menuItem2T);

    }

    subMenu2.eventListeners.add("onInvoke", menuItem2Handler);

    subSubMenu1 = app.scriptMenuActions.item(subMenuItem1T);

    if( subSubMenu1 == null ) {

        subSubMenu1 = app.scriptMenuActions.add(subMenuItem1T);

    }

    subSubMenu1.eventListeners.add("onInvoke", subMenuItem1Handler);

    subSubMenu2 = app.scriptMenuActions.item(subMenuItem2T);

    if( subSubMenu2 == null ) {

        subSubMenu2 = app.scriptMenuActions.add(subMenuItem2T);

    }

    subSubMenu2.eventListeners.add("onInvoke", subMenuItem2Handler);

    mnu = subs.item(menuT);

    if( mnu == null ) {

        mnu = subs.add(menuT, LocationOptions.after, refItem);

    }

    subsSubs = app.menus.item( '$ID/Main' ).submenus.item( menuT ).submenus;

    mnuSubMenu = subsSubs.item( subT );

    if( mnuSubMenu == null ) {

        mnuSubMenu = subsSubs.add( subT);

    }

    mnu.menuItems.add(subMenu1,LocationOptions.BEFORE,mnuSubMenu);

    mnu.menuSeparators.add(LocationOptions.BEFORE,mnuSubMenu);

    mnu.menuItems.add(subMenu2,LocationOptions.AFTER,mnuSubMenu);

    mnuSubMenu.menuItems.add(subSubMenu1,LocationOptions.AFTER,mnuSubMenu);

    mnuSubMenu.menuItems.add(subSubMenu2,LocationOptions.AFTER,mnuSubMenu);

};


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Guide ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Great.

I infer that my code did not work for you (although I don't know why).

OK, considering the final "Correct Answer," I just would like to point out that your event handlers (menuItem1Handler, menuItem2Handler...) are all unnecessary. You can directly supply a script File as second argument in the eventListeners.add() method. In addition, your code does not check whether the corresponding files actually exist, so it does not accordingly update the menu items if some feature is missing.

@+
Marc

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Hi Marc,

I wouldn't infer any such thing. Looking at your code, it probably works fine.

Chances are, my code was probably easier to read...

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Guide ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Harbs. wrote:

Chances are, my code was probably easier to read...

Well, in this area, I think "all is relative" but I'm not going to argue this specific point.

Now, let's suppose that someone wants to use your menu loader in another project having a different number of menu items and submenus. Say he wants to obtain the following hierarchy:

MyMenu

   MyFeature1
   MyFeature2

   MySubmenuA

      MyFeatureA1

      MyFeatureA2

      MyFeatureA3

   MySubmenuB

      MyFeatureB1

      MyFeatureB2

etc.

Do you see how many changes he needs to do in your code?

In the solution I suggest, he only has  to update the FEATURES array this way:

FEATURES = [
   { caption: "MyFeature1", fileName: "File-One.jsx", subName: "" },
   { caption: "MyFeature2", fileName: "File-Two.jsx", subName: "" },

   { caption: "MyFeatureA1", fileName: "File-A-One.jsx", subName: "MySubmenuA" },
   { caption: "MyFeatureA2", fileName: "File-A-Two.jsx", subName: "MySubmenuA" },
   { caption: "MyFeatureA3", fileName: "File-A-Three.jsx", subName: "MySubmenuA" },

   { caption: "MyFeatureB1", fileName: "File-B-One.jsx", subName: "MySubmenuB" },
   { caption: "MyFeatureB2", fileName: "File-B-Two.jsx", subName: "MySubmenuB" }

   ]

Is this less readable?

@+

Marc

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Like you said, it's relative. I personally find your code a pleasure to read, but my natural style is a bit different.

It really depends on your background.

If you have a strong OOP background, your code is pure poetry!

If you don't have an OOP background, it takes a lot of mental overhead to follow how the code is supposed to work.

Harbs

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

When I answer questions, I try to stick as closely to the coding style of the OP as I can, to help answer the question at hand. (It helps clarify the answer in few words -- and I don't enjoy excessive writing...)

Chances are I'm not going to change anyone's coding style in a single forum post...

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Hey, Marc: I'm going to be sort of blunt. Your code is hard for most people to read because it uses Javascript idioms they are entirely unfamiliar with, which don't make a lot of sense to people not really familiar with the language. And that's what most people here are. Let's take a walk through it:


(function(/*obj|undefined*/HOST)

The comment itself is confusing. Actually, it's even confusing to me.

    HOST || (HOST=$.global);

Tbe || idiom is confusing. Most people don't know what it means offhand.

    // ---
    // Prevent the current (startup) script from being uselessly rerun
    // ---
    if( HOST[$.engineName] )
        {
        alert( "This script is automatically executed at startup. You shouldn't run it manually." );

Well, this isn't confusing, but I think it's bad. One of the important things to do is to test what happens when the menus already exist, and that's easiest to do by rerunning the script.

    var MENU_NAME = "My Test Menu",

All caps for a function-local variable? Even using all caps for a globally scoped variable (Crockford) is confusing, but for a function-scope variable? That confuses me!

        FEATURES = [
            { caption: "My Menu Item 1", fileName: 'File-One.jsx', subName: "" },

Use of Objects is justified here, but they are probably confusing.

        FEATURE_LOCATION_PATH = (function()

Anonymous functions...definitely confusing! I'm also not sure what the point of this function heere is. Why not execute the code directly

on the next line?

            try{ f=app.activeScript; }
            catch(_){ f=File(_.fileName); }

Variables named "_"? Confusing!

            })();

And this is why anonymous functions are confusing! Not much to be done about it, but why use one where you don't need to. It's not like we're polluting the global namespace here.

    while( i-- )

Use of a while loop is a totally legitimate style choice, but I think it's typically less readable than a for loop. YMMV, of course!

        t = FEATURES;

I'm horribly guilty of this, but single-letter variable names are not the most readable.

            (t.action = app.scriptMenuActions.add( t.caption )).
                addEventListener('onInvoke', f);

More parentheses imply more mental gymnastics to understand. Breaking this into two lines,

with t.action=addEventListener... would help readability.

  var s,
        n = FEATURES.length,
        subs = {},
        sub = null;

Wait, a mid-function var block? Gasp! What's up with that? Crockford's right on this. One var statement and all at the top.

I am not sure why we're initializing sub to null -- what's wrong with the good old undefined? This probably doesn't qualify as confusing.

    for( i=0 ; i < n ; ++i )

Caching FEATURES.length in n is good for performance, but it probably doesn't matter here, and makes it more confusing to read. Not by much, but let's say half a confusion point. I would also say that "++i" is harder to read than "i++", maybe to the tune of 0.1 confusion-points, but maybe that's just me. YMMV Extremely.

        sub = (s=t.subName) ?
            ( subs
|| (subs=mnu.submenus.add( s, LO_END )) ) :
            mnu;

The ternary conditional operator, ?:, is pretty much always hard to read. And it's worse with short-circuit or-ing, ||, or three close-parens in rapid succession. Of course ?: tends to promote extra () as it almost always becomes ()?(): so...


        }
})();

And then it's worth re-emphasizing, basically nobody understands the anonymous function paradigm .

So there we have it, that's why I think your code is confusing to most readers.

(On the other hand Dan's final code ignores my recommendation and still has those abysmal ==null tests in there. What's up with that? )

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Guide ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

OK guys. Once again I admit my coding style has many faults and I really do not claim it is better than any other. It's just my way. I use it because I understand it better, I make it more easily evolve and I identify more easily the internal logic. But it's only because I used to do like that, and what is right for someone may definitely be wrong for someone else.

Anyway, my point is not about style. I'm just talking about factorization and reusability. Let down my code in itself, just consider the approach I suggest. What I mean is that the response considered correct has some issues and can be improved clearing useless event handlers, adding file checking, removing global vars and, why not, supporting reusability.

@+

Marc

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Marc, I did try your code. It worked at first, but once I edited it, the menu disappeared. I likely made a mistake which caused that. Now that you've explained a bit more of how to create a menu hierarchy with it, I'll try it again. It does sound like the best solution!

I wish I could mark two answers as correct, because I think both are valid answers. It was hard for me to decide which to say is "correct" and I hope I didn't offend you Marc. I only went with Harbs because it it's easier to read and understand the entire code (although, as you say, updating the main part of yours is easier to read and edit).


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Dec 18, 2011 Dec 18, 2011

Copy link to clipboard

Copied

Marc. Is it possible to insert menu dividers in your example? And if so, how?


— Adobe Certified Expert & Instructor at Noble Desktop | Web Developer, Designer, InDesign Scriptor

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines