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

Update logic block so it is reactive to the input attached to it. #53

Merged
merged 1 commit into from
Jan 29, 2015

Conversation

pkendall64
Copy link
Contributor

if you place a number in an input the other input changes it's type to number as well.
When you remove all inputs it reverts back to allowing any input type.

I really don't like the way this is done because there will be a listener added for each block of this type and it has to check all input connections. It would be preferable if the framework provided a call back when a connection was made or unmade to an input and provided the changed input as a parameter, as this would simplify the code quite considerably.

I also realize I could do this with a single listener that iterates all blocks and calls a method on the block, but that is basically the same code as here.

i.e. if you place a number in an input the other input changes it's type to number as well.
When you remove all inputs it reverts back to allowing any input type.
NeilFraser added a commit that referenced this pull request Jan 29, 2015
Update logic block so it is reactive to the input attached to it. (Refactoring coming shortly)
@NeilFraser NeilFraser merged commit 0512085 into google:master Jan 29, 2015
@NeilFraser
Copy link
Contributor

I've refactored your rather frightening code with this commit:
193c474

However, I wonder if this is the best solution. I foresee novice users getting frustrated that a block won't fit in the comparison operator for reasons that are not clear to them. Would it not be better to allow any block to connect, but pop out the opposing block if they don't match? That would certainly make it easier to edit code.

@pkendall64 pkendall64 deleted the logic branch January 29, 2015 01:43
@NeilFraser
Copy link
Contributor

Here's an alternative behaviour:
69294d8
What do you think?

@pkendall64
Copy link
Contributor Author

Yeah, that was some pretty awful code. I wasn't actually expecting it to be merged as-is!
Thanks for fixing it and changing to use the onchange callback, I could not find that anywhere for the life of me!
I was talking about the "pop the other block" scenario with a colleague and that solution is not ideal either as the user would wonder why his block popped out. Certainly for our application I think it's better to reject the connection of the second block.
I'm working on extending the Array support to allow generic arrays ala Java and have a "contains" block that updates the types accepted by the second input based on the generic array on the first input.

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