adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
851 stars 74 forks source link

Bug fix: document `change` event should be emitted before history `timetravel` #3685

Closed shaoshing closed 8 years ago

shaoshing commented 8 years ago

When timetravel event is fired, ColorInput requires its prop to be updated so that it can re-render based on the current props, and the update is triggered by document store's change event. Below is the expected working order:

  1. Undo command executed
  2. document store emits change event
  3. ColorInput receives updated props, but skip rendering
  4. history store emits timetravel event
  5. ColorInput handles timetravel and re-render

With our recent optimization on browser paint, document store's change event is delayed to allow browser starts rendering ASAP, and this hack disrupts the order, and now it becomes: 1 - 4 - 5 - 2 - 3

This PR use promise to ensure the correct event order.

Fix for #3658

@mcilroyc can you review and let me know if you approve the fix?

mcilroyc commented 8 years ago

This seems to fix the issue, and your description makes sense.

In reviewing this, it occurred to me that for the general use of setDocument there could be unintended consequences of adding the asynchronous delay prior to emitting the "change" event. All the event handlers that call setDocuments will now return before the event is emitted. That was introduced prior to this PR, and seems to be working anyway, so just noting it here for the heck of it.