[CLOSED] Issue #2076: Two indistinguishable events for different cases of working set reordering

[core-ai-bot]

Issue by TomMalbran
Saturday Jul 13, 2013 at 22:40 GMT
Originally opened as adobe/brackets#4450

---

Fix for #2076

With this, now both drag and drop reorder and the context menu sort methods trigger the same workingSetSort event and drag and drop uses it in a way that it doesn't redraw the DOM.

It would have been pretty easy to remove the workingSetDisableAutoSorting event and just use the workingSetSort event to cancel the automatic sorting, but since pull request #4437, save as can sometimes trigger a workingSetSort event, which would disable the automatic sort in some unwanted situations.

@peterflynn Could you take a look as this solution? I think this is what you wanted :)

---

TomMalbran included the following code: https://github.com/adobe/brackets/pull/4450/commits

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 16, 2013 at 20:53 GMT

---

I think that it will be best if we make a new workingSetRefresh event for Save As and then get rid of workingSetDisableAutoSorting and just use workingSetSort to disable the auto sorting.

[core-ai-bot]

Comment by peterflynn
Tuesday Jul 23, 2013 at 00:41 GMT

---

@TomMalbran Thanks for jumping on this!

Two things I think could make this cleaner:

• Trigger "workingSetSort" directly from DocumentManager.swapWorkingSetIndexes(). That eliminates the need for triggerWorkingSetSort().
• In WorkingSetView._handleWorkingSetSort(), use an internal guard flag to avoid unneeded _rebuildWorkingSet() calls -- i.e., set the flag before calling swapWorkingSetIndexes() and clear it afterward, and check the flag in _handleWorkingSetSort(). That eliminates the need to pass around a suppressRedraw flag (especially I'd prefer to keep the event itself free of flags that are meaningful only to one specific listener, if possible).
• Bonus third item :-) ... I think if you used a similar guard-flag approach in WorkingSetSort, you could eliminate the need for the "workingSetDisableAutoSorting" event.
  [edit: Oops, except for the point you made in PR [CLOSED] [Core][Inline color editor]: Inline color editor and quick view for colors can work in any type of files in brackets. #4523... we can't actually eliminate that event as long as addToWorkingSet() still uses "workingSetSort" for non-sorting cases. So we can skip this bullet for now.]

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 23, 2013 at 00:58 GMT

---

@peterflynn

• Sure will fix 1 and 2.
• For the third point: My current problem is that Save As sometimes triggers a workingSetSort. So I'll need to know when is triggered by Save as, vs a sort method. If Save As didn't use it, then I would have no problems in removing workingSetDisableAutoSorting. Should we just change Save As to use a new event? If someone wants to listen to workingSetSort events, they might get one sent when no reordering was done.

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 23, 2013 at 02:11 GMT

---

@peterflynn Fixes pushed. I added a few extra JSDoc fixes too.

As you noticed I couldn't get rid of the workingSetDisableAutoSorting yet. Once is no longer used by Save As, the changes to removed are trivial.

[core-ai-bot]

Comment by peterflynn
Tuesday Jul 23, 2013 at 18:30 GMT

---

@TomMalbran Looks great! Merging now...