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

Add/edit/delete global flow variables in UI #76

Merged
merged 13 commits into from
Apr 30, 2020

Conversation

reddigari
Copy link
Collaborator

Adds a menu as per Matt's wireframe where workflow-level variables are displayed and can be added, changed, and deleted.

Includes a change in the node config form component for numeric values to make sure they are passed as numbers in the JSON.

There seems to be a bug (not sure if it's here or on the back end) where the variables are not removed after refreshing the page (upon which the workflow SHOULD be reset). I'll keep looking into it.

Screen Shot 2020-04-28 at 9 54 57 AM

Copy link
Member

@reelmatt reelmatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I think I found the source of the "missing nodes" bug mentioned in #75; requesting those changes here because I think it fits with the PR.

I did notice the persistent global flow variables a few times, but not in any reproducible way. The closest I got was a Load > Save > Clear loop that seemed similar to the bug back in #28, but again, I couldn't reliably reproduce it. I thought re-fetching the globals would've solved it, but I'd say the current behavior as "good enough".

@@ -31,7 +33,9 @@ class Workspace extends React.Component {

componentDidMount() {
this.getAvailableNodes();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is the bug I mentioned in #75 about order-of-operations. Since the Node list is now tied to a Workflow object, that call needs to come after API.initWorkflow(). Moving it into the then() block seems to do the trick.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reddigari reddigari merged commit 1cf2809 into master Apr 30, 2020
@reelmatt reelmatt mentioned this pull request Apr 30, 2020
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