Skip to content

Conversation

@himdel
Copy link
Contributor

@himdel himdel commented Jun 28, 2018

(The first two commits are #4181.)

This adds a way for toolbar buttons to open modals somehow defined by the provider.

Added the button to Compute > Clouds > Providers list - first button - Magic.

Clicking it will fire a http request (right now, call_the_endpoint fakes it) to get additional data,
and based on the response, will render a react component (previously added to the registry by ManageIQ.component.addReact('name', ...)) or the angular dialog-user component (TODO).

Cc @martinpovolny , @Hyperkid123

@himdel
Copy link
Contributor Author

himdel commented Jun 28, 2018

Right now this works for react:

magic

(the styling is wrong for now because .modal-body > .container doesn't work, but removing the .container element fixes it)

@miq-bot miq-bot added the wip label Jun 28, 2018
himdel and others added 11 commits June 30, 2018 10:16
for now, the button is hardcoded to send the id of the first EmsCloud
this just listens for a message with controller=provider_dialogs

and triggers a fake endpoint call, that should return the details
…nd closable

closing the modal should be as easy as `const close = () => div.remove();`

But.. yay patternfly.
because creating and managing an "instance" when within another react component is just silly

this adds a simple `ManageIQ.component.getReact(name)` => the original react component passed to `addReact`
@himdel
Copy link
Contributor Author

himdel commented Jun 30, 2018

And almost there with dialog-user..

angular

(except right now, it's trying to render dialog-user multiple times)

EDIT: fixed, no more errors :)

@himdel
Copy link
Contributor Author

himdel commented Jun 30, 2018

So, @martinpovolny the important bits would seem to be ready,

What's probably missing right now is integrating the modal buttons with the form buttons. as that requires cooperation from the component.

Maybe we could use something like #3509 to separate the buttons from the component. Or maybe we can just add options to renderModal for now, and pass the button defs and callbacks there.

@miq-bot
Copy link
Member

miq-bot commented Jun 30, 2018

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/89ce342ceddd1a569957cc44a6e8e74d13a96f55~...1f05d27b71ab196600d0324add2cc0a30b82f3cd with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 4 offenses detected

app/helpers/application_helper/toolbar/ems_clouds_center.rb

  • ❗ - Line 12, Col 66 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 21, Col 66 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 22, Col 3 - Layout/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
  • ❗ - Line 3, Col 5 - Layout/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

@martinpovolny
Copy link
Member

martinpovolny commented Jul 2, 2018

@himdel : we don't need this to be done as Modal dialog, it can take the whole page if it's easier to do. Any of the 2 would work.

Well at least for now ;-)

@Hyperkid123
Copy link
Contributor

@martinpovolny that overflowing dialog is my fault. I thought that patternfly grid layout will work everywhere... it can be fixed very easily..

@martinpovolny
Copy link
Member

@Hyperkid123 : then, please, fix this and ping me in the PR. Thx!

@miq-bot
Copy link
Member

miq-bot commented Jul 9, 2018

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

@himdel
Copy link
Contributor Author

himdel commented Aug 10, 2018

Closing, all of this is merged as #4267 and #4315

@himdel himdel closed this Aug 10, 2018
@himdel himdel deleted the provider-dialogs branch August 10, 2018 16:27
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.

4 participants