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

Turn off 'open_url' if the button is to be displayed for a list #4838

Merged
merged 1 commit into from
Nov 2, 2018

Conversation

romanblanco
Copy link
Member

Problem is, that if the 'Open URL' is checked, and 'Display for' is changed to anything else than Single, the 'Open URL' checkbox gets disabled and it's not possible to uncheck it.

After these changes, the checkbox is automatically unchecked, if the button is supposed to be displayed for a list.

Also the checkbox will only be visible for a VM and Instance buttons, as it can only be applied there.

Steps for Testing/QA

  1. Automation - Automate - Customization - Buttons
  2. add a Custom Button to a VM and Instance model

@miq-bot
Copy link
Member

miq-bot commented Oct 26, 2018

Checked commits romanblanco/manageiq-ui-classic@2e41ed2~...cb3e301 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

- if @edit[:new][:disabled_open_url]
.note
= _("Only available for VM with \"Display for\" set to \"Single\"")
- if @resolve[:target_class] == "VM and Instance"
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't going to work, because content of @resolve[:target_class] will be translated into currently used locale.

You need to use more robust mechanism here, something that doesn't depend on an ad-hoc string.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, removed that part

@romanblanco romanblanco force-pushed the automate_button_checkbox branch from cb3e301 to 2e41ed2 Compare November 1, 2018 16:34
@mzazrivec mzazrivec added the bug label Nov 2, 2018
@mzazrivec mzazrivec self-assigned this Nov 2, 2018
@mzazrivec mzazrivec added this to the Sprint 98 Ending Nov 5, 2018 milestone Nov 2, 2018
@mzazrivec mzazrivec merged commit 86111ec into ManageIQ:master Nov 2, 2018
@mzazrivec
Copy link
Contributor

@romanblanco is this hammer/yes ?

@romanblanco romanblanco deleted the automate_button_checkbox branch November 2, 2018 10:16
@romanblanco
Copy link
Member Author

romanblanco commented Nov 2, 2018

@mzazrivec hammer/no

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.

3 participants