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

Basic form improvements #1210

Merged
merged 5 commits into from Oct 9, 2019
Merged

Conversation

andresmgot
Copy link
Contributor

This PR fixes some small issues:

  • Thanks to the fix of Blank lines and comments after null element get lost eemeli/yaml#125 we can know parse the values.yaml as a YAML Document without major changes. The new version 0.7.1 includes that fix.
  • In case the user changes the values in the "Advanced" form, the YAML is parsed and the basic parameters are recalculated to modify the basic form if needed.
  • We only show the basic/advanced tabs if the application has some basic parameters defined.

this.setState({
basicFormParameters: retrieveBasicFormParams(value, this.props.selected.schema),
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this will mean the basic form params are updated on every keystroke into the text box? Might be fine as is, I was just wondering if it was necessary (ie. could we just do this once when switching to the basic form).

Also, I've not checked, but assuming this doesn't create a loop (ie. if I update the basic form, it updates the values which triggers this method, which re-evaluates the basic parameters). From memory you're using the string value as the source of truth, so it may do (not quite a loop, but perhaps not wanted).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this will mean the basic form params are updated on every keystroke into the text box? Might be fine as is, I was just wondering if it was necessary (ie. could we just do this once when switching to the basic form).

Well, yes, it may be an issue if the values is big, I can move this logic whenever the Basic tab is clicked.

Also, I've not checked, but assuming this doesn't create a loop (ie. if I update the basic form, it updates the values which triggers this method, which re-evaluates the basic parameters). From memory you're using the string value as the source of truth, so it may do (not quite a loop, but perhaps not wanted).

Good point, there was indeed an unnecessary step.

@andresmgot andresmgot merged commit b7d7080 into vmware-tanzu:master Oct 9, 2019
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.

None yet

2 participants