adobe-photoshop / spaces-design

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

Stop listening to moveToArtboard events, and look for it in move events #3758

Closed baaygun closed 8 years ago

baaygun commented 8 years ago

Starting with pg-dev-mac#1104, we will get an array named moveToArtboard inside the move and duplicateNewSheets commands that will tell us all the layers that changed artboards with structure: [ { layer: #, artboard: # }, ...]

Whoever is reviewing, please do a quick testing as well (I've done some but it's nice to have extra eyes)

Also addresses #3748

iwehrman commented 8 years ago

The diff looks fine, but I ran across what may well be a distinct bug: img Note that when I drag the rectangle inside the artboard, it's Y coordinate changes to something that seems wrong, and then is different after I deselect and reselect the layer.

I'm using Mac pgdev.1105.

iwehrman commented 8 years ago

Anyway, just let me know if this is a completely separate issue, from what you can tell. If so, I'll file it and merge this. Back to @volfied.

iwehrman commented 8 years ago

Any thoughts on this @volfied?

baaygun commented 8 years ago

I've actually realized that my core changes are not correct. While testing, I missed on the fact that I was getting last drag event's artboard changes (due to old code emitting "move" event before posting it).

So I'm working on fixing that, and once I fix it hopefully these changes will be correct. Stay tuned.

baaygun commented 8 years ago

@chadrolfs @iwehrman with CL 1065232, pg-dev-mac 1117, this PR should correctly work.

I've explained the problem above, and fixed it with this new CL.

chadrolfs commented 8 years ago

looking at this branch with x1117, I still see issues with coordinates, specifically wrt Y, dragging out of then into an AB. ab-coordsoff

baaygun commented 8 years ago

@chadrolfs one more time, with build 1119, and update branch (I just pushed a new commit to this PR)

Now PS will send the events at the right time (after it's completely processed), and we also needed to relax TransformPanel's shouldComponentUpdate to check for selectedRelativeChildBounds so it knew to re-render the X/Y fields with the correct position after canvas shifted.

iwehrman commented 8 years ago

This gif shows some weird behavior with the Y coordinate of a layer outside of an artboard: img

chadrolfs commented 8 years ago

I noticed the same thing @iwehrman called out. Also a couple of minor issue, sometimes the policy doesn't update until you move the mouse and sometimes the policy will jump while moving. Here's a gif showing both of those minor issues.

ab-moveupdatepolicy

chadrolfs commented 8 years ago

ab-move

iwehrman commented 8 years ago

The policy delay probably has to do with the frequency with which PS sends us these move events. In any case, I think that should be benign?

baaygun commented 8 years ago

@chadrolfs Policy delay is due to us debouncing handleCanvasAutoResize events. The second gif is also due to same events, top level layers are counted like artboards in the sense that they will stretch/shrink the canvas. And this is done in tiles, that's why Y jumps from 1024 to 85 when you move it downwards.

At this point, none of these problems can be solved with JS, nor do I think they are in the scope of this PR and the bug it's attempting to fix.

iwehrman commented 8 years ago

OK!