Working Set Performance Improvements #9493
Conversation
|
@redmunds fixed failing unit test. This was also pointing to opening a project and the focus looked like it was in the editor but it really wasn't. |
JeffryBooher
changed the title
|
Ready for review. |
| $ghost.remove(); | ||
| $el.css("opacity", ""); | ||
| if (dragged) { | ||
| $("#working-set-list-container").removeClass("dragging"); |
There was a problem hiding this comment.
$("#working-set-list-container") is already stored in $workingFilesContainer, so no need for another lookup.
|
Done with code review. Behavior feels solid. All tests pass. Just a few comments. |
|
@redmunds ready for another review |
| @@ -470,6 +470,7 @@ define(function (require, exports, module) { | |||
| * Semi-private: should only be called within this module or by Document. | |||
| * @param {!Document} document Document whose main/full Editor to create | |||
| * @param {!Pane} pane Pane in which the editor will be hosted | |||
| @ @return {!Editor} | |||
There was a problem hiding this comment.
Nit: There is a @ instead of *.
| @@ -1239,14 +1249,16 @@ define(function (require, exports, module) { | |||
| } else { | |||
| DocumentManager.getDocumentForPath(file.fullPath) | |||
| .done(function (doc) { | |||
| _edit(paneId, doc, options); | |||
| _edit(paneId, doc, {noPaneActivate: true, | |||
| noPaneRedundancyCheck: true}); | |||
There was a problem hiding this comment.
This call to _edit() ignores the original options object that was passed in. These settings should instead be set on the original options object so that any other settings get passed through. Even if these are currently the only 2 possible options, some new ones might be added some day.
|
Done with review. A few more comments. |
|
@redmunds @TomMalbran thanks for the review! Let me know if it needs any further work before merging |
|
@JeffryBooher I just noticed a weird dragging case:
Results |
|
Closing in lieu of #9507 |
This is for #9439 -- Clicking on items in the working set is slow.
Pane.focus(one big drag wascurrentElement.blur()so we only need to do that if we're focusing something that isn't code mirror.EditorManager._showEditor(), the UX changes for the pane DOM weren't targeting the right node to see if it needed to re-add the editor to the pane so this was happening every time the editor was shown._openand_editto not activate pane until after opening the documentAlso fixes #9428