Skip to content
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

Curvifying edges #8

Merged
merged 9 commits into from
Jun 24, 2021
Merged

Curvifying edges #8

merged 9 commits into from
Jun 24, 2021

Conversation

alexlitty
Copy link
Collaborator

Prototype / demo PR that turns straight line edges into cubic bezier lines.

Branched off #6, includes changes from there too.

@alexlitty
Copy link
Collaborator Author

@maximecb I saw this on the to-do list and was excited to give it a try -- Here's my take on it.

A usual case: (also note how unconnected edges look)

Screen Shot 2021-06-22 at 12 23 03 PM

I think it handles decently at edge (hehe) cases, though it may not match your vision:

Screen Shot 2021-06-22 at 12 32 27 PM

I'm still figuring out what to do with nodes that are aligned like this:

Screen Shot 2021-06-22 at 12 43 50 PM

Thoughts?

@alexlitty alexlitty requested a review from maximecb June 22, 2021 19:49
public/svg.js Outdated Show resolved Hide resolved
@maximecb
Copy link
Owner

Groovy! I checked out the branch to try it and this looks really good. Works very well!

Two requests:

  • I think I would prefer if the SVG logic was directly in the edge class, not having a utility CubicLine class.
  • It would be nice if, when connecting from an output/src port, the edge immediately had the color matching the output port. What do you think?

@alexlitty
Copy link
Collaborator Author

alexlitty commented Jun 23, 2021

Sweet!

I think I would prefer if the SVG logic was directly in the edge class, not having a utility CubicLine class.

I think I would too. It's feeling redundant at hindsight, I'll port it over.

It would be nice if, when connecting from an output/src port, the edge immediately had the color matching the output port. What do you think?

Funny enough this is how it was working before, I "patched" it in 3871c95 :)

For me I saw a potential UX improvement -- Before 3871c95, when you connect an edge from the src to the dst, there's no visual feedback for the successful connection. You have to move your mouse away to "confirm" the edge was connected as you intended.

After 3871c95, all unconnected edges became gray, and all connected edges had a color; The change in color became visual feedback for the successful edge connection.

But I'm also with you, I liked the edge immediately having a color (behavior before 3871c95). Would you consider another kind of visual feedback independent of edge color? (We could make a new issue & pr for it and brainstorm in there)

@maximecb
Copy link
Owner

maximecb commented Jun 23, 2021

That's fair. It is nice to have visual feedback for edge connections. We could leave it as is with the edges slightly grey before they are connected. I will merge this PR once CubicLine is combined with the Edge class. We can always revisit visual feedback for connections later. You did really great work. I thought this would be hard to get right, but it works beautifully 👌

In terms of next things to work on, I'm starting to look at audio generation. I will try to make a push for that during the following days. Once the general setup is more in place, it will become clearer how the general design fits together and it will be easier for you to contribute to that.

Otherwise, some things you could work on next (as you prefer):

  • Ability to copy and paste one or more nodes
    • We probably also want to support the copy buffer so we can copy between tabs
    • This will allow people to copy things they find cool in other people's projects, including custom user-created modules
  • Node parameters menu
    • In Zupiter this shows up when double-clicking on a node
    • We should allow people to edit input and output name for module (group) nodes
  • User accounts, sharing projects and browsing project (the '/browse page)
    • I have some code for this from Zupiter, but it could use some polishing I think
    • This is something we really need in order to share this project with the public
  • Converting the Zupiter help page to markdown and linking to it in the menu (fun I know, lol)

Let me know what your preference would be and I'll open an issue so we can have a more detailed discussion :)

@alexlitty
Copy link
Collaborator Author

I appreciate the kind words a lot, thank you ☺️ And thank you for all the great back and forth, you're really awesome to work with.

For this PR: Sounds great. I'll get the cubic line code moved over here shortly, keep the current coloring as/is, and we'll revisit the feedback story.

For next steps: Excellent, I'm excited to see everything coming together! I'm having a great time working on pretty much anything -- Take your time with audio generation, make something you're happy with, and let me know if I can help at all on that specific front. Happy to be a rubber duck, etc.

Otherwise I'm happy to work on these in any order. A somewhat arbitrary order of preference based on context-switching:

  1. Copying and pasting nodes (I'll start looking at the copy/paste API and understanding what this will look like)
  2. Node parameters menu (Will this require the dialog changes story first? I saw you mentioned this in your original issue list)
  3. Help page (I saw your discussion in Help page #7, I think it would be a really nice touch having it available self-contained and offline -- But I understand there might be more in this discussion I'm not seeing, like the maintenance for keeping up with HTML docs)
  4. Browse page and its dependencies (accounts & sharing)

@maximecb
Copy link
Owner

Pleasure working with you as well. I'm always afraid people will get annoyed because I give a lot of feedback. Trying to balance my tendency for perfectionism with not getting into people's way too much.

I will open an issue for copying and pasting so we can discuss the approach and requirements :)

Node parameters, it would probably make sense for me to just bring over the code I have from Zupiter and we can try to improve it later. No need to start from zero. Could do the same for the help page. Just take the Zupiter help page and put it into a standalone HTML document 🤔

@alexlitty alexlitty requested a review from maximecb June 24, 2021 00:22
@maximecb maximecb merged commit 87e00c5 into main Jun 24, 2021
@maximecb maximecb deleted the litty/curved-edges branch June 24, 2021 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants