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

feat(deploy/blockDevices): Adds a Volume section in the Deploy wizard #8911

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

jorgebee65
Copy link
Contributor

@jorgebee65 jorgebee65 commented Feb 12, 2021

This feature adds a Volume section in the Deploy Wizard which allows to add one ore more volumes to an instance and also add Tags which applies to the list of volumes. Dependency on this other PR

volumes

@jorgebee65 jorgebee65 changed the title Block device tags feat(deploy/blockDevices): Adds a Volume section in the Deploy wizard Feb 12, 2021
@christopherthielen
Copy link
Contributor

This PR should use the FormikFormField system for handling form input and managing form state

Copy link
Contributor

@christopherthielen christopherthielen left a comment

Choose a reason for hiding this comment

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

thanks for migrating this to Formik. I have a couple minor requests then I think it's good to go.

Comment on lines +61 to +67
<div className="form-group">
<div className="sm-label-left">
<b>Tags (optional)</b>
<HelpField id="aws.serverGroup.blockDevice.tags" />
</div>
<MapEditor model={blockTags as any} allowEmpty={true} onChange={this.tagsChanged} />
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use a FormikFormField and a MapEditorInput which would also make the validate and `tagsChanged functions unnecessary

Comment on lines +22 to +31
public validate(
values: IBlockDevicesCommand,
): FormikErrors<IBlockDevicesCommand> {

const errors = {} as any;
const { blockDevices } = values;
console.log(blockDevices)
return errors;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this function gets called ever, and it is also a noop, so might as well remove it

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.

5 participants