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

[WIP] Remove dead miqSetButtons & miqButtonOnWhen #11760

Closed
wants to merge 1 commit into from
Closed

[WIP] Remove dead miqSetButtons & miqButtonOnWhen #11760

wants to merge 1 commit into from

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Oct 7, 2016

Since the toolbar refactor, miqSetButtons doesn't really do anything because the onwhen functionality is handled differently in the toolbar code (via Rx rowSelect).

Removing, thus making miqButtonOnWhen superfluous too.

WIP for now - because we either want to remove this, or change the toolbar to support a setCount action, which may be more robust than the current isClicked ? this.countSelected++ : this.countSelected--; (here, via) and use that instead.

Cc @martinpovolny, @karelhala

since the toolbar refactor, miqSetButtons doesn't really do anything because it's handled differently in the toolbar code (via Rx `rowSelect`)

Removing, thus making miqButtonOnWhen superfluous too.
@miq-bot
Copy link
Member

miq-bot commented Oct 7, 2016

Checked commit https://github.com/himdel/manageiq/commit/d2252dae8c4706b1dd83327943b3f8c8900f0cfd with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
4 files checked, 0 offenses detected
Everything looks good. 🏆

@himdel
Copy link
Contributor Author

himdel commented Oct 11, 2016

note to self: #11815 is introducing the "set toolbar count" part of this already..

@himdel
Copy link
Contributor Author

himdel commented Oct 11, 2016

another note to self: found the expected count bug...

go to http://localhost:3000/ems_infra/10000000000036?display=ems_clusters&vat=true
check 2 clusters, toolbar compare .. and then push the back button
those clusters are still checked, but the toolbar thinks 0, and uncheck/recheck goes to -2 and back to 0

@himdel
Copy link
Contributor Author

himdel commented Oct 11, 2016

Adding a setCount in ManageIQ/ui-components#33 - v0.0.8

@martinpovolny
Copy link
Member

watch out for: #11952

I consider @h-kataria 's patch a temporary one and we should get rid of miqSetButtons

possibly handle the RHN buttons in a different way (convert it to angular?)

@miq-bot
Copy link
Member

miq-bot commented Oct 17, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@chessbyte
Copy link
Member

Please re-create PR in https://github.com/ManageIQ/manageiq-ui-classic

@chessbyte chessbyte closed this Jan 11, 2017
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