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

Consider output error for connectability #1725

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RunDevelopment
Copy link
Member

Resolves #1673.

If PR makes it so that inputs are not connectable of the assigned type would cause an output error. So if the target node is valid (no output errors), but the new connections would create output errors, then we don't allow that connection. Examples:

image
image

While this enforces the general idea "if it's connectable, it's valid," it also makes it a little harder to edit chains.

E.g. in the Guided Upscale example, I initially had source and guide the other way around by accident. Chainner would not let me assign the other input with the intended value until I removed the old (invalid) connection.

Frankly, I don't know whether I like this. A new user might not realize that they have to remove old invalid connections before they can connect their intended value. This might "trap" new users and lead to frustration. But maybe I'm overthinking here.

Please test this feature and see whether this feature is desirable.

@joeyballentine
Copy link
Member

In general i think the idea is good, but yeah i can see how it might "trap" users. Maybe we could have a popup instead that says it's invalid, but give them the option to cancel or remove the other connections making it invalid?

@joeyballentine
Copy link
Member

Any more thoughts on what we should do with this?

@RunDevelopment
Copy link
Member Author

The user experience with the feature is not good. It's too restrictive.

Instead of disallowing connections that will cause errors, I think it might be better to allow it, but show a warning. Not sure about how though.
We could give the handle something like an orange warning color and show a tooltip on how. But our handles are already pretty colorful, so it might not be clear enough.

@joeyballentine
Copy link
Member

We could highlight the handle and edge with red maybe? We specifically haven't used red yet so it could be used for error stuff

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.

Input assignability should consider output type never.
2 participants