adobe-photoshop / spaces-design

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

Layers panel does not update when two images are placed #3750

Open DivyaPrabhakar opened 8 years ago

DivyaPrabhakar commented 8 years ago

When two image layers are dragged into DS and placed on an artboard, the layers panel does not reflect this until the user clicks on the layers panel. Both images are present, but the layers panel does not accurately reflect the state of the document. Note: I am dragging 2 images from my cloud linked Dropbox folder, this may play a part in the issue?

iwehrman commented 8 years ago

Could be related to #3635?

iwehrman commented 8 years ago

Can you verify that this is distinct from #3635 @DivyaPrabhakar?

ktaki commented 8 years ago

It probably different issue from #3635. The following error was reported in console when a layer in layers panel was clicked. Recommend to fix this along with #3635 for June since drag-and-drop of image files is a popular workflow.

screen shot 2016-03-07 at 2 00 07 pm

iwehrman commented 8 years ago

FBNC to @ktaki.

ktaki commented 8 years ago

@shaoshing: Placing multiple images succeeds. When one of the images are moved on canvas, however, you get a History warning.

[History] Re-initializing history because photoshop thinks the current state is 4 of 5, while our model says 5 of 6 [History] Initializing history with 5 states

This does not happen when single image file was placed.

Steps: Open new doc. Drag two files on to the canvas. Select the top image and drag.

shaoshing commented 8 years ago

@ktaki The warning is because when placing multiple files, we will receive multiple historyState events before receiving the placeEvent event, which result in incorrect count of history state. For example, when drag-and-drop two files, the order be like (number is the count of history state):

historyState (1)
historyState (2)
placeEvent <- expecting two placeEvent but only receive one
addLayer (2)
addLayer (3)

While the expected order should be:

historyState (1)
placeEvent
addLayer (1)
historyState (2)
placeEvent
addLayer (2)

To fix this complete, we will need the core team's help in fixing the event orders, but the good news is the warning can be safely ignored as we will reset the wrong state count to the correct one, and undo/redo will all work as expected. The downside is this will invalidate our history state cache which has some impact on performance when undoing.

screen shot 2016-03-11 at 11 03 42 am

CC @mcilroyc in case I am wrong and he can correct me.

ktaki commented 8 years ago

Thanks @shaoshing. I am removing Ship Blocker label.

mcilroyc commented 8 years ago

That sounds right @shaoshing.