aimeos / ai-admin-jqadm

Aimeos e-commerce Vue.js+Bootstrap based admin interface
https://aimeos.org
GNU Lesser General Public License v3.0
177 stars 43 forks source link

Multiple activated categories problem (+ reproduction steps + video) #119

Closed rowild closed 4 years ago

rowild commented 4 years ago

Environment

  1. Version 2019.10 and 20.4
  2. TYPO3 9.5.19
  3. php 7.3.18
  4. Mac os 10.13.6

Describe the bug There are cases, when there are suddenly multiple activated categories (in catalog tree). I was told, you are already aware of that problem, but was asked to find a reproduction szenario, which I think I did.

To Reproduce

  1. Click on a category
  2. Click on "categories" from the Aimeos menu
  3. Click on another category
  4. Click on "categories" from the Aimeos menu

Repeat steps 3 and 4 as often as you want.

On page reload, aimeos remembers the active categories and additionally adds active state to the root category.

To deactivate, simply click on an active category (quasi toggle).

Expected behavior Only one active category.

Screenshots A zip file is included, which contains a Camtasia video that showcases the problem, as well as a sql file with the catalog categories used in the video.

video-and-sql-file.zip

rowild commented 4 years ago

Adding some console.logs to Aimeos.Catalog.onClick shows that the window.location is not updated correctly:

// catalog.js
Aimeos.Catalog {

    onClick : function(event) {
        console.group('%cAimeos.Catalog.onclick', 'background: orange; color: black; padding: 4px 6px; border-radius: 3px;')
        console.log('event.node.id =', event.node.id);
        console.log('BEFORE assigning window.location: window.location.search =', window.location.search.split('&')[3]);
        window.location = $(".aimeos .item-catalog").data("geturl").replace("_ID_", event.node.id);
        console.log('AFTER assigning window.location:  window.location.search =', window.location.search.split('&')[3]);
        console.groupEnd()
    },
}

The 4th parameter of window.location.search shows either not the current id (this happens, when the Aimeos menu Categories is clicked for the first time after opening the page; the current id should be the 4th param of the search path or the window.location), or it shows the id that was previously active.

Not sure, if this is the root of the problem...

However, when clicking the "Categories" menu, the form's #idem-id always has no value, and the #item-parentid will always be set to the number of the rootid.

BTW: since you are using ids, which by W3C definition must be unique for a document, wouldn't it be enough to write

var id = $("#item-id").val() || $("#item-parentid").val();

instead of

var id = $(".aimeos .item-catalog #item-id").val() || $(".aimeos .item-catalog #item-parentid").val();

?

aimeos commented 4 years ago

We've removed some code for selecting the nodes automatically and added that to the new 19.10.7 release. Can you test if it works better now?

rowild commented 4 years ago

Unfortunately the problem still exists:

Screen Shot 2020-06-18 at 08 38 10

localStorage shows that jqTree's selected_node is an array. Is there a reason, why there are multiple values possible in a variable that should hold only one selected id? (Or is there a scenario, where multiple selected nodes are possible?)

Screen Shot 2020-06-18 at 08 37 51

EDIT: The updated jqTree v 1.4.12 is unchanged, I assume? EDIT 2: I tested with aimeos v20.4... sorry, need to test 19.10.7

EDIT 3: Tested with 19.10.7 - Problem remains.

But I just realised that this problem only occurs as soon as the tree has more then one level. Only then multiple selected categories are generated, and only on level 2. (I have a strong feeling that this is a jqTree problem?)

rowild commented 4 years ago

I'll check jqTree standalone and report back...

aimeos commented 4 years ago

Does the problem persist if you clear the local storage?

rowild commented 4 years ago

Yes, it does (on v19 and v20).

rowild commented 4 years ago

Sharing an observation: In 'catalog.js', around line 60, this line var node = root.tree("getNodeById", id); does not have a value as soon as a node on a deeper level then level 1 is clicked, even though id has aa value. (This is not true for categories on the first level. There node always has a value.)

aimeos commented 4 years ago

The second level and below are loaded dynamically when opened the first time (https://github.com/aimeos/ai-admin-jqadm/blob/master/admin/jqadm/themes/catalog.js#L111-L133) but the complete tree is built when clicking on an already loaded node AFAIK

rowild commented 4 years ago

That would mean that, when "Categories" from the Aimeos menu is clicked, no node info of second+ level categories is available, even though they might be set in localStorage, which is, why the rootId is activated again... right? So there should be a check, if there is a value in localStorage and use that one, if available, otherwise set to toor... right?

So var root = Aimeos.Catalog.createTree(Aimeos.Catalog.transformNodes(result)); must be a Promise... ?

rowild commented 4 years ago

An artifical promise with setTimeout works:

        var root = Aimeos.Catalog.createTree(Aimeos.Catalog.transformNodes(result));

        root.bind("tree.click", Aimeos.Catalog.onClick);
        root.bind("tree.move", Aimeos.Catalog.onMove);

        setTimeout(function (root) {

            var id = $(".aimeos .item-catalog #item-id").val() || $(".aimeos .item-catalog #item-parentid").val();
            var node = root.tree("getNodeById", id);

            console.log('id =', id);  // <-- always 1, when "categories" is clicked
            console.log('node =', node);

            if(!root.tree("getSelectedNode") && node !== undefined) {
                root.tree("selectNode", node);
            }

            if(node !== undefined) {
                console.log('invoke root.tree(openNode)');
                root.tree("openNode", node);
            }

        }, 500, root)

But that's nothing for production...

rowild commented 4 years ago

IMSORRYIMSORRYIMSORRY!!!!!

I implemented your fix wrong!

IT IS WORKING!!! THAAAAAAANK YOU!!!!!