1

muy buenas tardes, perdonen mi ignorancia apenas estoy comenzando en el mundo del diseño web y me seguí un tutorial para una pagina web pero en este paso ( que implementan un código de ) no se, si así deba escribirse me perdí porque al momento de que la persona lo "simplificó" ya no supe hacerlo y por mas que veo y veo no lo alcanzo a entender -----

este es el código:

function displaymenu() {
    var display;
    var card_menu = document.getElementById("card_menu");
    display = card_menu.style.display;

    if (display == "none") {
        card_menu.style.display = "block";
    }
    else {
        card_menu.style.display = "none";
    }
}

function navSection(nav) {

    var sections = new Array(5);
    sections[0] = "home";
    sections[1] = "sobre_mi";
    sections[2] = "skills";
    sections[3] = "portafolio";
    sections[4] = "blog";
    var search;
    var show;
    var bkgr_body;
    var bkgr_menu;

    for (var i = 0; i < 5; i++) {
        search = document.getElementById(sections[i]);
        search.style.display = "none";

        if (sections[i] == nav) {
            search.style.display = "block";

            if (sections[i] == "home") {
                bkgr_body = "#233d58";
                bkgr_menu = "#233d58";

            }
            else {
                bkgr_body = "#f1f1f1";
                bkgr_menu = "#29abe2";
            }
            search.style.background = bkgr_body;
            var menu = document.getElementsByTagName("header")[0];
            menu.style.background = bkgr_menu;
        }

    }
}

function displaySection(nav) {
    var sections = new Array(5);
    sections[0] = "home";
    sections[1] = "sobre_mi";
    sections[2] = "skills";
    sections[3] = "portafolio";
    sections[4] = "blog";

    var search;
    var show;


    for (var i = 0; i < 5; i++) {
        search = document.getElementById(sections[i]);
        show = search.style.display;

        if (show == "block") {
            search.style.display = "none";
            if (nav == "next") {
                i++;
                if (i > 4) i = 0;

            }
            if (nav == "prev") {
                i--;
                if (i < 0) i = 4;
            }
            search = document.getElementById(sections[i]);
            search.style.display = "block";
            break;
        }
    }
    //colocar s al final de section para que funcione bien
    if (sections[i] == "home") {
        bkgr_body = "#233d58";
        bkgr_menu = "#233d58";

    }
    else {
        bkgr_body = "#f1f1f1";
        bkgr_menu = "#29abe2";
    }
    search.style.background = bkgr_body;
    var menu = document.getElementsByTagName("header")[0];
    menu.style.background = bkgr_menu;
}

function displayformcontact(status) {
    var form = document.getElementById("contact_form");
    form.style.display = status;
}

la persona que lo realizo decia simplificarlo pues al parecer hay acciones que se repiten y borro algunas cosas y puso otras para que sea menos codigo... Nuevamente perdonen mi poco talento apenas voy aprendiendo pero si me gustaria poder corregir ese detalle....la accion que realiza si funciona la esta haciendo pero deseo simplificarlo como esta persona mencionaba y no he podido hacerlo al tratar de mover y quitar cosas ya no me funciona. :-( ¿alguien que pueda orientarme por favor?

Pablo Lozano
  • 45,934
  • 7
  • 48
  • 87
Ed Edd Eddy
  • 11
  • 2

3 Answers3

2

El código en general se ve bien, lo único que cambiaría es lo siguiente:

Para inicializar arrays puedes usar la notación de corchetes, ahorrando en claridad. Además, usar el constructor es innecesario, por lo que:

var sections = new Array(5);
sections[0] = "home";
sections[1] = "sobre_mi";
sections[2] = "skills";
sections[3] = "portafolio";
sections[4] = "blog";

se puede transformar a:

const sections = ["home", "sobre_mi", "skills", "portafolio", "blog"];

Además defines en dos sitios el mismo array, así que puedes omitir la segunda definición si lo creas como constante global.

Otro detalle es que no es buena idea modificar el contador de un bucle for dentro del cuerpo del mismo, pero como en este caso se produce un break cuando se modifica, no es un problema.

Por otro lado, dejaría de usar var y usaría let y const siempre.

Yo prefiero usar comillas simples en los strings en lugar de dobles, porque así puedo meter código HTML en un string usando comillas dobles en los atributos, pero esto ya es más una cuestión de gustos, así que el código quedaría algo así (no lo he probado):

function displaymenu() {
    let card_menu = document.getElementById('card_menu');
    if (card_menu.style.display == 'none') {
        card_menu.style.display = 'block';
    } else {
        card_menu.style.display = 'none';
    }
}

const sections = ['home', 'sobre_mi', 'skills', 'portafolio', 'blog'];

function navSection(nav) {

    let search;
    const bkgr_body;
    const bkgr_menu;

    for (let i = 0; i < 5; i++) {
        search = document.getElementById(sections[i]);
        search.style.display = 'none';

        if (sections[i] == nav) {
            search.style.display = 'block';

            if (sections[i] == 'home') {
                bkgr_body = '#233d58';
                bkgr_menu = '#233d58';

            } else {
                bkgr_body = '#f1f1f1';
                bkgr_menu = '#29abe2';
            }
            search.style.background = bkgr_body;
            const menu = document.getElementsByTagName('header')[0];
            menu.style.background = bkgr_menu;
        }
    }
}

function displaySection(nav) {
    let search;
    let show;
    let i = 0;

    for (; i < 5; i++) {
        search = document.getElementById(sections[i]);
        show = search.style.display;

        search = document.getElementById(sections[i]);
        show = search.style.display;

        if (show == 'block') {
            search.style.display = 'none';
            if (nav == "next") {
                i++;
                if (i > 4) i = 0;

            } else if (nav == "prev") {
                 i--;
                 if (i < 0) i = 4;
            }
            search = document.getElementById(sections[i]);
            search.style.display = 'block';
            break;
        }
    }
    // colocar s al final de section para que funcione bien
    if (sections[i] == 'home') {
        bkgr_body = '#233d58';
        bkgr_menu = '#233d58';
    } else {
        bkgr_body = '#f1f1f1';
        bkgr_menu = '#29abe2';
    }
    search.style.background = bkgr_body;
    let menu = document.getElementsByTagName('header')[0];
    menu.style.background = bkgr_menu;
}

function displayformcontact(status) {
    let form = document.getElementById('contact_form');
    form.style.display = status;
}
Pablo Lozano
  • 45,934
  • 7
  • 48
  • 87
  • Muchas gracias, probare con este código, espero sea lo que se necesita, nuevamente te agradezco mucho. y luego te comento si funciono "gracias" – Ed Edd Eddy Jul 19 '19 at 21:44
1

empezando por el principio, yo te diría primero que identifiques las piezas de código repetidas, por ejemplo, tu array sections lo declaras varias veces en distintas funciones y le asignas siempre los mismos valores. Además, primero te creas un array de cinco posiciones y luego asignas, la forma más sencilla de escribirlo es sacarlo a una constante fuera del scope de las funciones que lo usan:

const sections = [
    "home",
    "sobre_mi",
    "skills",
    "portafolio",
    "blog",
];

Luego, en tu función displayMenu primero declaras las variables display y card_menu y luego les asignas el valor. Realmente podrías, de nuevo sacarlas a constantes (esta vez dentro del scope de la función) y asignarles el valor directamente. En cuanto a la reasignación de estilo, podrías hacer una reasignación condicional con una ternaria.

function displaymenu() {
    const card_menu = document.getElementById('card_menu');
    card_menu.style.display = card_menu.style.display === 'none' ? 'block' : 'none';
}

Con respecto a navSection podrías aplicar los consejos que te di antes, usando asignaciones ternarias y, además, como tienes una asignación compartida con displaySection te recomiendo que saques eso a una función. Además, te sugiero iterar usando el método forEach de Array.

function getBackgroundValue(el) {
    return el === 'home' ? {
        body: '#233d58',
        menu: '#29abe2',
    } : {
        body: '#f1f1f1',
        menu: '#233d58',
    };
}

function navSection(nav) {

    sections.forEach(el => {
        const search = document.getElementById(el);
        const show = search.style.display;

        search.style.display = el === nav ? 'block' : 'none';

        if (el === nav) {
            const menu = document.getElementsByTagName('header')[0];

            const bgValue = getBackgroundValue(el);
            search.style.background = bgValue.body;
            menu.style.background = bgValue.menu;
        }
    });

}

Aplicando los mismos principios, obtenemos la función displaySection, que quedaría así:

function displaySection(nav) {
    const menu = dosument.getElementsByTagName('header')[0];
    const el;
    for (let i = 0; i < sections.length; i++) { 
        const search = document.getElementById(sections[i]);
        const show = search.style.display;

       search.style.display = show === 'block' ? 'none' : show;

        if(show === 'block' && (nav === 'next')) {
            i++;
            if (i > 4) i = 0; 
        } else if (show === 'block' && (nav === 'prev')) {
            i--;
            if (i < 0) i = 4;
        };

        el = sections[i];
        search = document.getElementById(el);
        search.style.display = "block";
        if(show === 'block') break;
   }
   const bgValue = getBackgroundValue(el);
    search.style.background = bgValue.body;
    menu.style.background = bgValue.menu;
}

displayformcontact() no tiene mucho donde reducir. Mas allá de que quieras hacer la asignación directamente.

El código quedaría tal que así:

const sections = [ "home", "sobre_mi", "skills", "portafolio", "blog"];

function displaymenu() {
    const card_menu = document.getElementById('card_menu');
    card_menu.style.display = card_menu.style.display === 'none' ? 'block' : 'none';
}

function navSection(nav) {

    sections.forEach(el => {
        const search = document.getElementById(el);
        const show = search.style.display;

        search.style.display = el === nav ? 'block' : 'none';

        if (el === nav) {
            const menu = document.getElementsByTagName('header')[0];

            const bgValue = getBackgroundValue(el);
            search.style.background = bgValue.body;
            menu.style.background = bgValue.menu;
        }
    });

}

function displaySection(nav) {
    const menu = dosument.getElementsByTagName('header')[0];
    const el;
    for (let i = 0; i < sections.length; i++) { 
        const search = document.getElementById(sections[i]);
        const show = search.style.display;

       search.style.display = show === 'block' ? 'none' : show;

        if(show === 'block' && (nav === 'next')) {
            i++;
            if (i > 4) i = 0; 
        } else if (show === 'block' && (nav === 'prev')) {
            i--;
            if (i < 0) i = 4;
        };

        el = sections[i];
        search = document.getElementById(el);
        search.style.display = "block";
        if(show === 'block') break;
   }
   const bgValue = getBackgroundValue(el);
    search.style.background = bgValue.body;
    menu.style.background = bgValue.menu;
}

function displayformcontact(status) {
    var form = document.getElementById("contact_form");
    form.style.display = status;
}

function getBackgroundValue(el) {
    return el === 'home' ? {
        body: '#233d58',
        menu: '#29abe2',
    } : {
        body: '#f1f1f1',
        menu: '#233d58',
    };
}

Se ha reducido a 68 líneas, que una reducción de más del 30% de líneas.

Fernando Carrascosa
  • 1,748
  • 3
  • 15
  • Muchas gracias, probare con este código, espero sea lo que se necesita, nuevamente te agradezco mucho. y luego te comento si funciono "gracias" – Ed Edd Eddy Jul 19 '19 at 21:44
0

He podido reducirlo unas 20 lineas, aunque tampoco se en que momento se llama a cada función, por lo que es muy limitado en lo que te puedo ayudar

var sections,menu;
function displaymenu(){
    var card_menu = document.getElementById("card_menu");
    var display=card_menu.style.display;
    if (display == "none"){
        card_menu.style.display ="block";
    }
    else{
        card_menu.style.display ="none";
    }
}
function navSection(nav){
    menu = document.getElementsByTagName("header")[0];
    sections = new Array("hombe","sobre_mi","skills","portafolio","blog");
    var bkgr_body,bkgr_menu;
    for (var i=0; i<5; i++){
        var search = document.getElementById(sections[i]);
        search.style.display = "none";
        if (sections[i] == nav){
            search.style.display = "block";

            if (sections[i] == "home"){
                bkgr_body = "#233d58";
                bkgr_menu = "#233d58";

            }
            else{
                bkgr_body = "#f1f1f1";
                bkgr_menu = "#29abe2";
            }
            search.style.background = bkgr_body;
            menu.style.background = bkgr_menu;
        }

    }
}
function displaySection(nav){
    for (var i=0; i<5; i++){
        var search = document.getElementById(sections[i]);
        var show = search.style.display;
        if (show == "block"){
            search.style.display = "none";
            if (nav == "next"){
                i++;
                if (i>4){i=0};
            }
            if (nav == "prev"){
                i--;
                if (i<0){i = 4};
            }
            search = document.getElementById(sections[i]);
            search.style.display = "block";
            break;
        }
    }
    //colocar s al final de section para que funcione bien
    if (sections[i] == "home"){
        bkgr_body = "#233d58";
        bkgr_menu = "#233d58";
    }
    else{
        bkgr_body = "#f1f1f1";
        bkgr_menu = "#29abe2";
    }
    search.style.background = bkgr_body;
    menu.style.background = bkgr_menu;
}
function displayformcontact(status){
    var form = document.getElementById("contact_form");
    form.style.display = status;
}

La verdad es que esa persona tenia un poco de razón de que hay que quitar cosas, si te fijas en tu código declaras varias variables con el mismo valor, como pueden ser menu y sections.

Mi recomendación es generar una función en el body con el onload para cargar las funciones globales (Aunque la declaración tiene que estar fuera de las funciones).
La razón del onload es que javascript se carga antes que los elementos de html por lo que si quieres "coger" un elemento de la página web (ejemplo document.getElementById("id_test")) no mostraría nada ya que no ha cargado aún el documento con los elementos

Jose
  • 1,009
  • 1
  • 4
  • 15
  • 1
    Muchas gracias, probare con este código, espero sea lo que se necesita, nuevamente te agradezco mucho. y luego te comento si funciono "gracias" – Ed Edd Eddy Jul 19 '19 at 21:44