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

Introduce support for multi-tab orchestration dialogs #17342

Merged
merged 1 commit into from
May 8, 2018

Conversation

miha-plesko
Copy link
Contributor

@miha-plesko miha-plesko commented Apr 25, 2018

Until now it was hard-coded that orchestration service dialog uses a single tab only, named "Basic Information". For some providers, however, this is not too convinient because there are many fields in dialog that can be grouped into multiple tabs.

With this commit we allow provider to decide how many tabs to use and what content to put in each tab. New feature is optional and fully backwards compatible.

Usage: should provider decide to define custom tabs, it can now override function tabs that returns a list of tab specifications.

Following screenshot demonstrates how VMware vCloud provider defined three tabs (Basic Information, vApp Networks and Instances)

capture

Original PR: #17289 (@gasper-vrhovsek left company)
Related PR: ManageIQ/manageiq-providers-vmware#242 (relies on this core PR being merged)
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1571758

@miq-bot assign @bzwei
@miq-bot add_label enhancement,provisioning,gaprindashvili/yes

/cc @agrare


template.parameter_groups.each_with_index do |parameter_group, index|
add_parameter_group(parameter_group, tab, index + 1)
if template.respond_to?(:parameter_groups_tabbed)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not test respond_to? for backward compatibility. The proper way is to introduce parameter_groups_tabbed method in the base template model. Default implementation is to provide a single tab named Basic Information, and allow the subclass to override with multiple tabs.

end

Array(tab_data[:param_group]).each_with_index do |param_group, i|
if defined? param_group.parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, can we avoid a test for defined?

Until now it was hard-coded that orchestration service dialog uses
a single tab only, named "Basic Information". For some providers, however,
this is not too convinient because there are many fields in dialog
that can be grouped into multiple tabs.

With this commit we allow provider to decide how many tabs to use
and what content to put in each tab. New feature is optional and fully
backwards compatible.

Usage: should provider decide to define custom tabs, it can now override
function `tabs` that returns a list of tab specifications.

Signed-off-by: Gašper Vrhovšek <gasper.vrhovsek@gmail.com>
Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
@miq-bot
Copy link
Member

miq-bot commented May 3, 2018

Checked commit miha-plesko@2472424 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 🏆

@miha-plesko
Copy link
Contributor Author

Sorry for delay @bzwei, was on PTO. Many thanks for the review, I like your suggestion about introducing common function in the base model. I've also updated the rspec tests to be more readable now. Looking forward to your opinion.

@bzwei
Copy link
Contributor

bzwei commented May 7, 2018

Looks good. Thanks.

@agrare agrare assigned agrare and unassigned bzwei May 8, 2018
Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍 nice change @miha-plesko

@agrare agrare merged commit fd76353 into ManageIQ:master May 8, 2018
@jrafanie jrafanie added this to the Sprint 86 Ending May 21, 2018 milestone May 22, 2018
simaishi pushed a commit that referenced this pull request May 30, 2018
Introduce support for multi-tab orchestration dialogs
(cherry picked from commit fd76353)

https://bugzilla.redhat.com/show_bug.cgi?id=1584296
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 6fc38238c6cf7f514443784cab9731ddd7ba8e49
Author: Adam Grare <agrare@redhat.com>
Date:   Tue May 8 08:22:38 2018 -0400

    Merge pull request #17342 from miha-plesko/tabbed-dialog
    
    Introduce support for multi-tab orchestration dialogs
    (cherry picked from commit fd7635350a48e9ab1a99f2348f42e8b1f5bc0160)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1584296

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.

7 participants