WebCabin / wcDocker

wcDocker (Web Cabin Docker) is a powerful window layout system with a responsive and completely interactive design. Move, remove, create, and duplicate panel windows at any time! Organize how you wish! View the demo here:
http://docker.webcabin.org
146 stars 53 forks source link

Using moveable(false) causes Browser window hang #123

Open mzabuawala opened 6 years ago

mzabuawala commented 6 years ago

Hi, I am frequently facing hangs if I set moveable to false as myPanel.moveable(false); After trying to move Panel ~4-5 times page becomes unusable.

I suspect some memory leak in the wcDocker code.

If I remove the myPanel.moveable(false); code then it is working fine, no hangs nothing.

mzabuawala commented 6 years ago

Found the code which is causing the issue,

It is an infinite loop when both pane0 & pane1 becomes true.

__findInner: function () {
            function isPaneStatic(pane) {
                return !!(pane && (pane.instanceOf('wcFrame') && pane.panel() && !pane.panel().moveable()) || (pane.instanceOf('wcCollapser')));
            }

            var parent = this._root;
            while (parent) {
                if (parent.instanceOf('wcSplitter')) {
                    var pane0 = isPaneStatic(parent._pane[0]);
                    var pane1 = isPaneStatic(parent._pane[1]);
                    if (pane0 && !pane1) {
                        parent = parent._pane[1];
                    } else if (pane1 && !pane0) {
                        parent = parent._pane[0];
                    } else if (!pane0 && !pane1) {
                        break;
                    }
                } else {
                    break;
                }
            }

            return parent;
        },
Lochemage commented 6 years ago

Preventing a panel from being movable is very tricky. A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation.

How exactly are you trying to use this feature?

mzabuawala commented 6 years ago

Here is the code snippet,

    panels: {
      // Panel to keep the left hand browser tree
      'browser': new pgAdmin.Browser.Panel({
        name: 'browser',
        title: gettext('Browser'),
        showTitle: true,
        isCloseable: false,
        isPrivate: true,
        isFramePanel: true,
        icon: 'fa fa-binoculars',
        content: '<div id="tree" class="aciTree"></div>',
      }),
      // Properties of the object node
      'properties': new pgAdmin.Browser.Panel({
        name: 'properties',
        title: gettext('Properties'),
        icon: 'fa fa-cogs',
        width: 500,
        isCloseable: false,
        isPrivate: true,
        elContainer: true,
        content: '<div class="obj_properties"><div class="alert alert-info pg-panel-message">' + select_object_msg + '</div></div>',
        events: panelEvents,
        onCreate: function(myPanel, $container) {
          $container.addClass('pg-no-overflow');
        },
      }),
      // Statistics of the object
      'statistics': new pgAdmin.Browser.Panel({
        name: 'statistics',
        title: gettext('Statistics'),
        icon: 'fa fa-line-chart',
        width: 500,
        isCloseable: false,
        isPrivate: true,
        content: '<div><div class="alert alert-info pg-panel-message pg-panel-statistics-message">' + select_object_msg + '</div><div class="pg-panel-statistics-container hidden"></div></div>',
        events: panelEvents,
      }),
      // Reversed engineered SQL for the object
      'sql': new pgAdmin.Browser.Panel({
        name: 'sql',
        title: gettext('SQL'),
        icon: 'fa fa-file-text-o',
        width: 500,
        isCloseable: false,
        isPrivate: true,
        content: '<div class="sql_textarea"><textarea id="sql-textarea" name="sql-textarea"></textarea></div>',
      }),
      // Dependencies of the object
      'dependencies': new pgAdmin.Browser.Panel({
        name: 'dependencies',
        title: gettext('Dependencies'),
        icon: 'fa fa-hand-o-up',
        width: 500,
        isCloseable: false,
        isPrivate: true,
        content: '<div><div class="alert alert-info pg-panel-message pg-panel-depends-message">' + select_object_msg + '</div><div class="pg-panel-depends-container hidden"></div></div>',
        events: panelEvents,
      }),
      // Dependents of the object
      'dependents': new pgAdmin.Browser.Panel({
        name: 'dependents',
        title: gettext('Dependents'),
        icon: 'fa fa-hand-o-down',
        width: 500,
        isCloseable: false,
        isPrivate: true,
        content: '<div><div class="alert alert-info pg-panel-message pg-panel-depends-message">' + select_object_msg + '</div><div class="pg-panel-depends-container hidden"></div></div>',
        events: panelEvents,
      }),
    },
...
...
...
    // Preparing the layout
    buildDefaultLayout: function() {
      var browserPanel = this.docker.addPanel('browser', wcDocker.DOCK.LEFT);
      var dashboardPanel = this.docker.addPanel(
        'dashboard', wcDocker.DOCK.RIGHT, browserPanel);
      this.docker.addPanel('properties', wcDocker.DOCK.STACKED, dashboardPanel, {
        tabOrientation: wcDocker.TAB.TOP,
      });
      this.docker.addPanel('sql', wcDocker.DOCK.STACKED, dashboardPanel);
      this.docker.addPanel(
        'statistics', wcDocker.DOCK.STACKED, dashboardPanel);
      this.docker.addPanel(
        'dependencies', wcDocker.DOCK.STACKED, dashboardPanel);
      this.docker.addPanel(
        'dependents', wcDocker.DOCK.STACKED, dashboardPanel);
    },

Full code: https://github.com/postgres/pgadmin4/blob/master/web/pgadmin/browser/static/js/panel.js

https://github.com/postgres/pgadmin4/blob/master/web/pgadmin/browser/static/js/browser.js +106, +251

mzabuawala commented 6 years ago

We have a feature request to make layout fixed.

FYI, the issue occurs when I try to click on Browser panel after setting it to .moveable(false). I also have one question even if I am creating as a Panel it shows up as Frame.

screen shot 2018-04-02 at 5 30 01 pm
Lochemage commented 6 years ago

All panels are contained within a wcFrame, this is in case you split your panel into multiple tabs.

As for disabling movement of all panels, this was not what wcDocker was designed for so there is no built in feature for this. However, if you are willing to make changes in the source code, you can disable movement of any panel by making sure the event system never begins the move process. On line 1845 of docker.js it tests if the panel should start moving (https://github.com/WebCabin/wcDocker/blob/master/Code/docker.js#L1845) If you set shouldMove to false then panels will not move.