-
-
Notifications
You must be signed in to change notification settings - Fork 287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve drag & drop / canvas UI #1553
Conversation
Does it look ok in dark mode? |
That is a nice exploration! The introduced change (blue boundaries) is blending really well. But it makes me think about the essence of showing them in the first place. It makes them critical as these guide in visualizing the layout of all components together so maybe we should keep it and anyway preview is just a click away, so it should not be a dealbreaker. About choosing bright red as the color for the highlighted component, it is a somewhat unconventional. Wondering if there could be a different tone of red/pink that we should explore. |
Red is traditionally used to indicate errors, so I do understand wanting a different color there. I do like @apedroferreira's use of the blue to tie in nicely with the theme. Light modeDark mode |
@gerdadesign There is another related construct in the canvas, the cursor that appears in between elements when dragging a component. I feel like it makes sense to make it part of this design to make all these construction elements tie together well visually |
Yeah, right now there is also a bright green color when dragging & dropping or resizing, initially I made it that color just so that it was easy to see and work with, but it doesn't blend in very well with the overall theme...
I'm fine with trying out only showing the borders on hover if most people are in favor. Not sure if it will be an improvement or not, but sounds like worth trying. |
By "on hover" what is meant exactly?
I'm in favour for the last option |
I think @gerdadesign meant the first option, but to me it doesn't sound bad to try 1 and 4 together. |
1 and 4 together seems good to me too. |
Yeah, those are kinda red so maybe we should change them too. |
Yes, I intended "on hover" to mean "hover over the element shows its outline". If you think it's also valuable to show all outlines while dragging a new element over, I'm down to see how that interaction would play out! @apedroferreira I also went with a 2px border vs 1px. Especially when dashed, it didn't feel visible enough to me. I'm open to changing my mind when seeing this in implementation vs a mockup, though. Here's a few screenshots with a little more detail: Currentexisting experience for comparisonProposalDefault canvas viewWhile hovering over an element
While resizing an element
Dragging a component over from the library |
@gerdadesign These look great, thanks! I'll give them a try and also try to implement the suggested options 1 and 4 to see how it feels. |
Actually there's also instead of 4: show all potential cursor positions when dragging a component. I think this makes more sense. |
Might be more difficult to implement but sounds more intuitive, I'll try that too as a preferred alternative to "dragging over the canvas shows all outlines". |
1ed758e
to
b2378c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally the screaming red is gone 😅
However, I've found something not working as expected with overlay 👇
CleanShot.2023-02-06.at.14.40.11.mp4
Were you trying to move an element here but it was selecting another element instead? Edit because don't want to add another comment: you're right, it is related after all because this PR introduced it, so will fix here! |
As seen in recording, I:
If you can find a way to fix this great, but in the mean time I'm retracting rejection if this isn't related to your changes and giving approval as the changes look good anyways 👍 |
It looks much better than bright red! Since there is a predominant blue on the canvas, I think both of the side panels should have the same color as we have now (something for the next cycle) Let's also take sign off from @gerdadesign |
This is looking really nice, @apedroferreira! 👏 |
i experimented a bit with trying to show the drop area triangles but it feels a bit weird sometimes so probably gonna leave it out - I'll leave it in the commit history here anyway. Screen.Recording.2023-02-06.at.19.44.48.mov |
I can see how drop triangle shadows impact DX, definitely something we can explore later. |
packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/NodeDropArea.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small suggestion on code style. lgtm otherwise 👍
UI redesign for canvas and drag-and-drop.
I had a brief attempt at "showing all potential cursor positions when dragging a component" but it would be quite a lot of work, and after trying to see how it would look I don't think it would look good with the current system - there would be too many cursor positions very close to each other and it would probably look pretty confusing...
Screen.Recording.2023-02-03.at.18.57.13.mov
Preview: https://toolpad-pr-1553.onrender.com/
Design