dynamic drop-down menu

Discussion in 'Javascript' started by scaredemz@gmail.com, Dec 11, 2005.

  1. Guest

    hi,
    so i'm creating a dynamic drop-down menu. the menu and the text show up
    fine in IE but only the drop-down shows in Firefox without the menu
    text. Below is the fxn code. help pls.

    function DropDownHelper(menuArray, top, left, height)
    {
    var currItem = new String();
    var item;
    var idStr;


    for(var i = 0; i < menuArray.length; i++)
    {
    currItem = menuArray;
    currItemArr = currItem.split(";");

    item = document.createElement("div");
    idStr = "dropDownMenuId" + i;

    ExistingMenuItemIds = idStr;

    item.setAttribute('id', idStr);
    item.style.visibility = "visible";
    item.style.top = top;
    item.style.fontFamily = "Arial";
    item.style.fontSize = "13pt";
    item.style.color = "white";
    item.style.fontWeight = "bold";
    item.style.position = "absolute";
    item.style.height = height;
    item.style.left = left;
    item.style.width = "200px";
    item.style.backgroundColor = "red";
    item.style.color = "white";
    item.style.cursor = "hand";

    item.innerHTML = "<p onclick = \"GoTo('" + currItemArr[1] + "')\">" +
    currItemArr[0] + "</p>";


    document.body.appendChild(item);
    var newtop = GetIntFromProperty(top) + height;
    top = newtop + "px";
    }
    }
    , Dec 11, 2005
    #1
    1. Advertising

  2. web.dev Guest

    wrote:
    > hi,
    > so i'm creating a dynamic drop-down menu. the menu and the text show up
    > fine in IE but only the drop-down shows in Firefox without the menu
    > text. Below is the fxn code. help pls.


    You have given us a function that you use. It would also be helpful to
    see how you're calling this function, that is, the values that are
    being passed in. Otherwise, the code that you've shown is perfectly
    valid.


    > item.setAttribute('id', idStr);


    Just to be consistent with your portion of the code, you can also set
    the id as follows:

    item.id = idStr;

    > item.style.visibility = "visible";
    > item.style.top = top;
    > item.style.fontFamily = "Arial";
    > item.style.fontSize = "13pt";
    > item.style.color = "white";
    > item.style.fontWeight = "bold";
    > item.style.position = "absolute";
    > item.style.width = "200px";
    > item.style.backgroundColor = "red";
    > item.style.color = "white";
    > item.style.cursor = "hand";


    See all that above you wrote? You can easily put that into a
    stylesheet and just refer to it by classname. It just saves you the
    trouble of having to programmatically do it everytime. Plus it helps
    separate presentation from logic.

    item.className = "styleName";

    > item.innerHTML = "<p onclick = \"GoTo('" + currItemArr[1] + "')\">" +
    > currItemArr[0] + "</p>";


    >From the problem you described, this seems to be the place where you're

    placing the text. You should check to see that there is indeed a value
    in currItemArr[0]. Though you do say it works in IE, the code you've
    given doesn't show how you're obtaining the value for menuArray. So it
    may not work in FF like you said.
    web.dev, Dec 11, 2005
    #2
    1. Advertising

  3. wrote:

    > so i'm creating a dynamic drop-down menu. the menu and the text show up
    > fine in IE but only the drop-down shows in Firefox without the menu
    > text. Below is the fxn code. help pls.
    >
    > function DropDownHelper(menuArray, top, left, height)
    > {
    > var currItem = new String();


    That is nonsense. JavaScript is not Java. You do not have to create
    a String object in order to store string values in a variable.

    > var item;
    > var idStr;
    >
    >
    > for(var i = 0; i < menuArray.length; i++)


    for (var i = 0, len = menuArray.length; i < len; i++)

    > {
    > currItem = menuArray;


    Here you are overwriting the reference that you retrieved at the top.

    > currItemArr = currItem.split(";");
    >
    > item = document.createElement("div");
    > idStr = "dropDownMenuId" + i;
    >
    > ExistingMenuItemIds = idStr;


    Unless `ExistingMenuItemIds' is intended to be a constructor function, its
    identifier should be changed so that it's first letter is lowercase. That
    is not a question of syntax, but of code style.

    > item.setAttribute('id', idStr);


    setAttribute() has too many flawed implementations to be considered a viable
    DOM method on the Web. And it is not necessary, HTMLDivElements have an
    `id' attribute that is exposed as an `id' property in DOM implementations.

    item.id = idStr;

    > item.style.visibility = "visible";


    You should make sure that there is a `style' property before you try to
    access properties that are properties of a supposed-to-be CSS2Properties
    object.

    > item.style.top = top;


    Whether that is viable depends on what you pass as value for `top'.

    > item.style.fontFamily = "Arial";


    The font-family property value should be a comma-separated list of font
    family names, ending with that of a _generic_ one, here "sans-serif".

    > item.style.fontSize = "13pt";


    `pt' is a unit suited for printouts, not for the screen. Use `%' or `em'.

    > [...]
    > item.style.height = height;
    > item.style.left = left;


    See above.

    > [...]
    > item.style.cursor = "hand";


    "hand" is not a Valid value for the `cursor' property in CSS(2), and will
    not work in Gecko-based browsers. The standards compliant equivalent for
    this IE-proprietary value is `pointer'.

    > item.innerHTML = "<p onclick = \"GoTo('" + currItemArr[1] + "')\">" +
    > currItemArr[0] + "</p>";


    `innerHTML' is a proprietary property that is supported in many browsers,
    but not in all, and is still not standards compliant and therefore
    error-prone.

    Additionally, you are abusing the p(aragraph) element as control. Use an
    `input' element instead; if you do not like it's style, use CSS to change
    that.

    > document.body.appendChild(item);


    Care to test methods before you call them?

    <URL:http://pointedears.de/scripts/test/whatami>

    > var newtop = GetIntFromProperty(top) + height;
    > top = newtop + "px";


    GetIntFromProperty(top) can probably be replaced easily by
    parseInt(top, 10).

    Are you aware that this menu will not work in client-side script or the
    required DOM support is not present?

    BTW: Do not use tabs but use spaces for indenting code, especially when
    posting to the newsgroup. Every system and user agent has its own
    interpretation of the Tab character.


    HTH

    PointedEars
    Thomas 'PointedEars' Lahn, Dec 11, 2005
    #3
  4. RobG Guest

    wrote:
    > hi,
    > so i'm creating a dynamic drop-down menu. the menu and the text show up
    > fine in IE but only the drop-down shows in Firefox without the menu
    > text. Below is the fxn code. help pls.
    >
    > function DropDownHelper(menuArray, top, left, height)
    > {
    > var currItem = new String();


    Please don't use tabs in posted code. Use 2 or 4 spaces for indents and
    wrap lines manually at about 70 characters.

    There is no need for a string object, you can initialise it as a string
    using:

    var currItem = '';


    > var item;
    > var idStr;
    >
    >
    > for(var i = 0; i < menuArray.length; i++)
    > {
    > currItem = menuArray;
    > currItemArr = currItem.split(";");


    This will create currItemArr as a global variable, is that neccessary?

    var currItemArr = currItem.split(";");


    >
    > item = document.createElement("div");


    You should test for support before using DOM features:

    if (document.createElement) {
    document.createElement('div');
    // ...


    > idStr = "dropDownMenuId" + i;
    >
    > ExistingMenuItemIds = idStr;


    Where was ExistingMenuItemIds declared? Is it a global variable?


    >
    > item.setAttribute('id', idStr);


    Simpler to use:

    item.id = idStr;


    > item.style.visibility = "visible";


    You should test for support for style objects, using a class is much
    simpler. Div's are visibile by defalut, there is not need to set them
    as visible unless you have previously set it to hidden.

    if (item.style){
    item.style.visibility = 'visible';


    > item.style.top = top;


    Where was 'top' defined? What is its value?


    > item.style.fontFamily = "Arial";
    > item.style.fontSize = "13pt";


    Font sizes should be set using proportional units - em, en, etc.


    > item.style.color = "white";
    > item.style.fontWeight = "bold";
    > item.style.position = "absolute";
    > item.style.height = height;
    > item.style.left = left;


    Where are 'height' and 'left' defined?


    > item.style.width = "200px";
    > item.style.backgroundColor = "red";
    > item.style.color = "white";


    That's the second time you've set the color attribute.


    > item.style.cursor = "hand";


    'hand' is not a valid CSS value for the the cursor, use 'pointer'
    (though I think some older versions of IE may require 'hand').


    >
    > item.innerHTML = "<p onclick = \"GoTo('" + currItemArr[1] + "')\">" +
    > currItemArr[0] + "</p>";


    You've used DOM for everything else, why not here too? It's a much
    safer way of adding the onclick:

    var oP = document.createElement('p');
    oP.appendChild(document.createTextNode(currItemArr[0]));
    oP.onclick = function(){GoTo(currItemArr[1])};


    >
    >
    > document.body.appendChild(item);
    > var newtop = GetIntFromProperty(top) + height;


    What does 'GetIntFromProperty()' do? Presumably it trims 'px' from the
    value of 'top'. Why not set top as an integer and concatenate 'px' when
    you set the style property?

    Where you declare your other variables, add:

    var newtop = 0;


    Presumably you want to set the div's 'top' attribute as:

    item.style.top = newtop + 'px';


    And here:

    newtop += top;


    > top = newtop + "px";
    > }
    > }
    >


    I can't test the code because you haven't given sufficient information.
    There are thousands of drop-down menus out there, many based on CSS
    that use very little JavaScript, some use none. Quite a few are offered
    completely unencumbered.

    Why not just use one of those? There are quite a few offered here:

    <URL:http://www.alistapart.com/topics/code/css/>



    --
    Rob
    RobG, Dec 11, 2005
    #4
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. SirPoonga
    Replies:
    2
    Views:
    759
    Ben Strackany
    Jan 7, 2005
  2. weiwei
    Replies:
    0
    Views:
    1,014
    weiwei
    Jan 5, 2007
  3. msimmons
    Replies:
    0
    Views:
    473
    msimmons
    Jul 16, 2009
  4. Replies:
    5
    Views:
    267
  5. Replies:
    3
    Views:
    285
Loading...

Share This Page