amiletti / spapp

Simple single page jquery application library
8 stars 4 forks source link

Issue with Multiple Sections Loaded When Changing Routes #2

Open AlbaraHassan opened 7 months ago

AlbaraHassan commented 7 months ago

Issue with Multiple Sections Loaded When Changing Routes

Description: When using the spapp library to manage routes in our web application, we've encountered a problem where multiple sections are loaded when changing routes. This issue results in forms being misaligned or not functioning properly.

Steps to Reproduce:

  1. Load the web application.
  2. Navigate between different routes using the spapp library.
  3. Observe that multiple sections are loaded simultaneously, causing forms to behave unexpectedly.

Expected Behavior: When changing routes, only the relevant section should be loaded, ensuring that forms and other components function correctly without interference from other sections.

Actual Behavior: Multiple sections are loaded simultaneously when changing routes, leading to misaligned forms and other issues in the application.

Code Snippet:


var routeChange = function() {
    var id = location.hash.slice(1);
    var route = routes[id];
    var elm = $("#" + id);
    var prevSection = $(".spapp-created")

    if (!elm || !route) {
        if (config.pageNotFound) {
            window.location.hash = config.pageNotFound;
            return;
        }
        console.log(id + " not defined");
        return;
    }
//Proposed fix
    prevSection.empty();
    prevSection.removeClass("spapp-created");

    if (elm.hasClass("spapp-created")) {
        route.onReady();
    } else {
        elm.addClass("spapp-created");
        if (!route.load) {
            route.onCreate();
            route.onReady();
        } else {
            elm.load(config.templateDir + route.load, function() {
                route.onCreate();
                route.onReady();
            });
        }
    }
}
amiletti commented 7 months ago

Hi Albara,

Thanks for your suggestion. This library was written 8 years ago... at time i think that the main reason why i don't delete the view at route change is for performance. If you need to reload the entire view on route change your solution is perfect.

If you want, and you have time to do ancd check this, you can add a config value like "reloadView" in config in order to be able to unload or not the view. In this case I appreciate if you do a push

// in config
config = $.extend({
    defaultView  : $("main#spapp > section:last-child").attr("id"),
    templateDir  : './tpl/',
    pageNotFound : false,
    reloadView   : false
}, options );}

// routeChange method
if(config.reloadView) {
    prevSection.empty();
    prevSection.removeClass("spapp-created");
}

In this way you can switch between two behavoirs by simply change the config

var app = $.spapp({pageNotFound : 'error_404', reloadView: true}); // initialize

In every way thanks for your suggestion

AlbaraHassan commented 7 months ago

I have created a pull request, and I made the changes as you had suggested

Hi Albara,

Thanks for your suggestion. This library was written 8 years ago... at time i think that the main reason why i don't delete the view at route change is for performance. If you need to reload the entire view on route change your solution is perfect.

If you want, and you have time to do ancd check this, you can add a config value like "reloadView" in config in order to be able to unload or not the view. In this case I appreciate if you do a push

// in config
config = $.extend({
    defaultView  : $("main#spapp > section:last-child").attr("id"),
    templateDir  : './tpl/',
    pageNotFound : false,
    reloadView   : false
}, options );}

// routeChange method
if(config.reloadView) {
    prevSection.empty();
    prevSection.removeClass("spapp-created");
}

In this way you can switch between two behavoirs by simply change the config

var app = $.spapp({pageNotFound : 'error_404', reloadView: true}); // initialize

In every way thanks for your suggestion