Skip to content

Conversation

skateman
Copy link
Member

@skateman skateman commented Feb 21, 2020

Depends on: ManageIQ/manageiq-api#741 ManageIQ/manageiq-api#830
Replaces: #6402
Parent issue: ManageIQ/manageiq#18818

The following providers are already supported:

Known issues:

@miq-bot add_label react, enhancement, compute/cloud

Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

So far so good. I am excited about the tests 😉

@skateman
Copy link
Member Author

I'm sure I'm not able to write tests, so I will rely on the experts 😁 @brumik can you help me in a followup pr?

@brumik
Copy link

brumik commented Feb 25, 2020

I think so :)

@skateman skateman force-pushed the cloud-provider-forms branch from 16d83e1 to 551ab3c Compare February 28, 2020 20:50
@Hyperkid123
Copy link
Contributor

@skateman you can use https://data-driven-forms.org/renderer/global-component-props if you want to override global props.

@skateman
Copy link
Member Author

@skateman you can use https://data-driven-forms.org/renderer/global-component-props if you want to override global props.

That's great, but does this work on DDFv1?

@Hyperkid123
Copy link
Contributor

@skateman oh right sorry. No its v2 feature. I keep forgetting.

@miq-bot
Copy link
Member

miq-bot commented May 27, 2020

This pull request is not mergeable. Please rebase and repush.

@skateman skateman force-pushed the cloud-provider-forms branch from b39068f to d8febd3 Compare May 27, 2020 15:09
@skateman skateman changed the title [WIP] Replace cloud provider forms with an API-driven DDF approach Replace cloud provider forms with an API-driven DDF approach May 27, 2020
@skateman skateman requested a review from h-kataria May 27, 2020 15:10
@miq-bot miq-bot removed the wip label May 27, 2020
@h-kataria
Copy link
Contributor

h-kataria commented May 27, 2020

@skateman

  • Add new Provider form should have "Add/Cancel" buttons instead of "Save/Reset/Cancel", to be consistent with other forms in the product
  • Flash message is missing when after changes in the Edit provider form are Reset
  • Just wondering if form buttons should be disabled while Validation is in process on Edit provider form.

@skateman
Copy link
Member Author

@skateman

  • Add new Provider form should have "Add/Cancel" buttons instead of "Save/Reset/Cancel", to be consistent with other forms in the product

Fixed

  • Flash message is missing when after changes in the Edit provider form are Reset

I'm not entirely sure why is this necessary, the reset doesn't do a server roundtrip anymore so there's no way it would fail due to a server error.

  • Just wondering if form buttons should be disabled while Validation is in process on Edit provider form.

Maybe it's possible to do it, but I'd like to take it into a separate PR and especially after we are on DDFv2.

@skateman skateman force-pushed the cloud-provider-forms branch from d8febd3 to 603a578 Compare May 28, 2020 10:32
@h-kataria
Copy link
Contributor

@skateman

  • Add new Provider form should have "Add/Cancel" buttons instead of "Save/Reset/Cancel", to be consistent with other forms in the product

Fixed

  • Flash message is missing when after changes in the Edit provider form are Reset

I'm not entirely sure why is this necessary, the reset doesn't do a server roundtrip anymore so there's no way it would fail due to a server error.

  • Just wondering if form buttons should be disabled while Validation is in process on Edit provider form.

Maybe it's possible to do it, but I'd like to take it into a separate PR and especially after we are on DDFv2.

I feel the Save should not be allowed while credentials are being validated, that will give user an opportunity to change/fix them in case validation failed. Let's do the disabling of Save button in a separate PR

I am not sure about removing the Reset flash message, we do have that in other DDF forms tho, is there a plan to remove it from all new forms?

@skateman skateman force-pushed the cloud-provider-forms branch from 603a578 to 4e9d756 Compare May 28, 2020 14:59
@skateman
Copy link
Member Author

Okay, I'll put it there...but will create an issue about getting rid of this.

@miq-bot
Copy link
Member

miq-bot commented May 28, 2020

Checked commits skateman/manageiq-ui-classic@48e04b3~...4e9d756 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
3 files checked, 3 offenses detected

app/views/ems_cloud/edit.html.haml

  • ⚠️ - Line 1 - Layout/TrailingBlankLines: Final newline missing.
  • ⚠️ - Line 1 - Line is too long. [190/160]

app/views/ems_cloud/new.html.haml

  • ⚠️ - Line 3 - Layout/TrailingBlankLines: Final newline missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants