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

Disable toolbar dropdown if no item in the group is enabled. #357

Merged
merged 2 commits into from Nov 5, 2018

Conversation

martinpovolny
Copy link

@@ -52,6 +52,12 @@ export class ToolbarListController implements IToolbarListBindings {
this.toolbarList.items &&
this.toolbarList.items.filter((item: IToolbarItem) => !item.hidden).length > 0;
}

private isToolbarEnabled(): boolean {
return this.toolbarList &&
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add && this.toolbarList.enabled to move the whole condition here?

Copy link
Contributor

@karelhala karelhala Nov 1, 2018

Choose a reason for hiding this comment

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

Yup, that sounds better, plus use some so we don't have to iterate over the whole array.

private isToolbarEnabled(): boolean {
  return this.toolbarList && 
    this.toolbarList.enabled &&
    this.toolbarList.items &&
    this.toolbarList.items.some((item: IToolbarItem) => item.enabled)
}

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I c-n-ped your method above. So fixing both methods to use some.

@JPrause
Copy link
Member

JPrause commented Oct 31, 2018

@miq-bot add_label blocker

@martinpovolny
Copy link
Author

@himdel, @karelhala : please, re-review. Thx!

@miq-bot
Copy link
Member

miq-bot commented Nov 3, 2018

Checked commits f224062~...1fbfd1d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@himdel himdel self-assigned this Nov 5, 2018
@himdel himdel added this to the Sprint 98 Ending Nov 5, 2018 milestone Nov 5, 2018
@himdel himdel merged commit af0d121 into master Nov 5, 2018
@himdel himdel deleted the toolbar_dropdown_disable branch November 5, 2018 12:49
@himdel
Copy link
Contributor

himdel commented Nov 5, 2018

LGTM :)

@himdel himdel restored the toolbar_dropdown_disable branch November 5, 2018 12:49
simaishi pushed a commit that referenced this pull request Nov 5, 2018
Disable toolbar dropdown if no item in the group is enabled.

(cherry picked from commit af0d121)

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

simaishi commented Nov 5, 2018

Hammer backport details:

$ git log -1
commit 793afc679b23760cdd7a7d14a6fda03eb2ccc09d
Author: Martin Hradil <himdel@seznam.cz>
Date:   Mon Nov 5 13:49:24 2018 +0100

    Merge pull request #357 from ManageIQ/toolbar_dropdown_disable
    
    Disable toolbar dropdown if no item in the group is enabled.
    
    (cherry picked from commit af0d12192f73942ffabd39dd67698085f9e3ce0b)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1633727

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.

None yet

6 participants