Community
Participate
Working Groups
we currently create the "object" scoped commands in the dom for each row in the navigator, and hide/show them on hover. The hover code is managed by the client, so the command framework has no idea that these elements are not always needed. This makes the DOM bigger, not just with simple image nodes, but also dijit submenus. Approaches for improvement: - have the client destroy the rendered commands rather than simply hide the dom node that contains them. Since some of the commands are dijit drop down menus we would have to detect this and destroy the digit widgetry (digitry?) along with the dom nodes. Bogus to have the client care how we rendered something. - add lifecycle methods to the command framework so that the client destroys rendered commands at a dom node and the framework knows what to look for - in the case of the drop-down menus, they could be shared across rows and repopulated in the same way you would manage a dynamic context menu. Would have to balance the response time it takes to compute and create everything on the fly vs. DOM bulk. For example, maybe we just optimize the menu case and not worry so much about image elements. Populating "object" scoped menus dynamically would also fix bug 338887 for the object, scoped commands, but we would still have to fix that bug for the drop down menu on the nav toolbar.
this is going to get worse as we have more plug-ins contributing nav commands, especially if there are validation rules to compute. For M6, I'm not going to put the move/copy commands at the object level because there is just too much computation going on regarding valid move/copy targets.
Gosia and I chatted about this. Here is the problem. - commands can have an href callback, and we support the href callback returning a promise. Originally this was to support plug-ins being able to calculate hrefs. - now we have the git remote command, which is a slightly different flavor. The item in the navigator does not carry the git remote location info in it, so the href callback has to do a GET just to get the URL that should be used to view the remote. This results in a GET on every visible item in the navigator. There are some long term issues to consider as well, but it is not likely we could get any of these implemented and tested for M7: 1 - it probably makes more sense in the long run that GIT could adorn the JSON item with the git remotes information, rather than have the client code have to go back and request it. This would make the performance issue go away. 2 - fixing bug 338888 would address *when* the computations happen. The discussion there is to only get the command info on hover or on menu open. This reduces the number of GETS because only items hovered or selected do the computation, but it could cause some noticeable delays in hover or menu operation. 3 - it is possible, (but not common and not yet implemented), that a project would be connected to multiple remotes. So using the href command for a remote is not a long term UI solution. Another possibility would be to include a list of remotes in a submenu, but we would have the same response time problem here if #1 was not implemented. Or maybe we go to a remotes page that lists the possible remotes. 4 - we have grown *a lot* of navigator commands in M7 and I'd like to do some reorganization and grouping during M8. However it'd be nice to do this from a usability standpoint, not to work around performance problems. All that said, Gosia and I discussed some workarounds for M7: 1 - the href returned by the command is a placeholder page that shows a progress message and does the GET for the actual URL. It loads the new page when the address is obtained. 2 - we move the git remote command to the "more actions" menu so that the gets are only done when the menu opens. Neither of us likes solving a perf problem with UI change, but this could be done. 3 - replace the href callback in the command with a normal callback that computes the URL and then switches the page. This ensures that the GET is only done when the user actually clicks on the link. The downside is that we lose the link context menu behavior because the href is not really known in the anchor link. 4 - a variant of #3 could be done inside the command framework so that we don't get the href until click. We'd still have links and the click/ctrl-click behavior, but we'd lose the link context menu behavior since the href is not known until click. Of all these possibilities, I like #1. To me, it feels like the git remote command is going to grow into a "list remotes" page anyway. So this placeholder page is kind of like a "list remotes" that figures out there is only one and redirects to the info about that one remote.
ignore the comment below, that was meant for a different bug. sigh.
For M8 the pragmatic solution seems to be to focus on populating menus dynamically. For example, we could process the commands as we do now to determine that a menu is needed, but perhaps we would not actually add menuitems until the menu opened. The menu open lifecycle is needed anyway for bug 341522. It also makes sense to investigate this after the move to dojo 1.6 and ensure any solution works well on IE9
removing milestone. Too risky for this point in the release and no profiling has proven it to be an issue (yet). It will be. A good candidate for early in next release.
See also bug 358296
Things are a lot different than when I first opened this bug, and we've "fixed" this bug by making the invisible items go away in bug 340615. We now need all command dom elements to be in the page. We also need the drop down menus to be visible. In bug 366258 we are beginning to move more of the item level commands into the menu, to reduce clutter, so there should be less elements. So the only question really remaining from this bug is whether we should be populating the menus as we process them, or wait until the menu opens. (When to pay). Since we have to process all the items regardless (to determine if a menu is needed at all), we may as well put the items in the menu. Otherwise we have to either: - recompute all that same state when the user pushes the menu button - save that state somewhere so that we can quickly generate the menu In other words, the expensive part needs to be done anyway (it's also used for remembering context for key bindings, URL bindings, etc.) So my original worry that we are generating a lot of DOM content for invisible things is no longer valid. I'm sure we could optimize the processing of commands, but I suspect that this could be done by tuning the command framework, not by storing more state info and delaying menu creation.
