Disallow invalid layer reorder placements as siblings of artboards#3289
Disallow invalid layer reorder placements as siblings of artboards#3289Sahil-Gupta584 wants to merge 11 commits intoGraphiteEditor:masterfrom
Conversation
|
Upon watching your video demo, I realized that the ideal behavior will be rejecting the placement opportunity in places that are siblings of artboards (generalized to be describable as "invalid placements"). This would need to be given to the frontend by the backend, or maybe better, answered by the backend in response to a query by the frontend. This way, we never show the white insertion line of a width that would indicate an invalid placement, which is a flaw of this "just correct it if it's invalid" approach that I realized now. |
makes sense. |
|
I'd suggest iterating on this PR with the suggested change of approach. No need to open a separate PR. |
Can you tell me more clearly what i need to implement? got little confused. and what about the feat for which pr is raised ? |
|
This PR. I'm suggesting you keep working on changes to it to implement my described updated approach (from my first comment in this discussion). |
Yes but what about dropping on bottom, should I consider it as valid or invalid now? |
|
I'm not understanding the question. All invalid drop locations shouldn't be valid white horizontal line placements, thus pre-filtering instead of post-rejecting. |
Keavon
changed the title
Ok so you mean dropping a layer as artboard sibling should be invalid? and currently if we try to do that we do see white line indicating its valid drop even though its invalid |
|
Correct. |
Sahil-Gupta584
force-pushed
the
enhance-artboard-layer-drop
branch
2 times, most recently
from
89aaa5e to
1ab4d41
Compare
Sahil-Gupta584
force-pushed
the
enhance-artboard-layer-drop
branch
2 times, most recently
from
510f06e to
2a61db5
Compare
|
Demo: Graphite.and.7.more.pages.-.Personal.2.-.Microsoft_.Edge.2025-10-25.22-37-17.mp4 |
|
Hey @Keavon you can have a look |
|
Your video seems to show the line jumping around and appearing in places rather erratically. Please pay attention to small details like that and ensure it's behaving how it should. Happy to take another look at a video with your next shot (and thanks for making it easy with the video recording for me). Thanks. |
|
Hey @Keavon, Hows its now? Graphite.-.Personal.2.-.Microsoft_.Edge.2025-10-31.22-31-12.mp4 |
|
That looks good! However at the bottom, it needs to check if a valid placement can exist and choose that one (the outermost if there are multiple). |
actually i dont find any reason to disallow layers/artbords placement at bottom most. |
|
No, only artboards are valid at the bottom of the artboard layer stack. |
|
@Keavon Is this one perfect? Graphite.and.3.more.pages.-.Personal.2.-.Microsoft_.Edge.2025-11-01.21-46-28.mp4 |
…nto enhance-artboard-layer-drop
timon-schelling
force-pushed
the
master
branch
from
124235a to
a42cad8
Compare
|
@Keavon @timon-schelling, just pinning. |
| const lastLayerDepth = layers[layers.length - 1]?.entry?.depth; | ||
| const draggingLayerDepth = layers[dataIndex]?.entry?.depth; | ||
|
|
||
| if (clientY > treeChildren[layers.length - 1].getBoundingClientRect().bottom - 10) { |
There was a problem hiding this comment.
why this exact 10 px threshold value is used ?
There was a problem hiding this comment.
also, move it to the top in a constant
There was a problem hiding this comment.
Its for better ui feel when user drags layer on edges. other wise use sometimes still sees white line for invalid drops.
|
This generally isn't working in a lot of ways: capture_20_.mp4 |
|
weird let me check again |
|
@Keavon Can you test again, tried directly using insertDepth |
|
If you pass CI and mark it as ready for review, I'll take a look then. |
3 participants
Closes #3256
Demo:
Graphite.-.Personal.2.-.Microsoft_.Edge.2025-10-15.20-04-11.mp4