-
Notifications
You must be signed in to change notification settings - Fork 356
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
Remove paging (not paging_div) after leaving GTL screens #3262
Remove paging (not paging_div) after leaving GTL screens #3262
Conversation
…a ReportController instance on screens without GTL, we may still need to clean the paging toolbar setExtraClasses (without params) does the right thing .. but assumes an instance created a function that runs it with a fake instance https://bugzilla.redhat.com/show_bug.cgi?id=1512957
…o clean up after GTL this is for ajax transitions from gtl screens to screens without gtl but with paging_div this should probably replace `.hide('pc_div_1')` everywhere
…:pc_div_1) pc_div_#(number) used to be a div under paging_div but above the pagination code this was removed with the GTL refactor thus, attempts to show/hide pc_div_1 are dead code, but indicate that those were the places where paging was being hidden replaced by explicit remove_paging Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1512957
pc_div_.. used to wrap the paging under paging_div, before the GTL rewrite replaced hiding it by remove_paging, removed code showing it (because GTL automatically creates it anyway)
Checked commits https://github.com/himdel/manageiq-ui-classic/compare/69419c02ce2bcd596e5539ed0e42403e100df447~...a7838821342b0ab0c28ebf65b2789e4435c91f5e with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
@karelhala @martinpovolny can you please review/merge this blocker PR. |
Testing in the UI, following these steps:
@himdel are you certain it was showing a paging bar on Diagnostics/server/Workers prior to this fix? |
@himdel : please, deal with the feedback in a follow-up PR if needed. Thx! |
@dclarizio Yes I am, but I also notice that there is still a fix needed .. if you go to Workers and then reload, the toolbar is there. But if you go to Settings first, and then switch to Workers, it's not. ( |
The problem where |
Remove paging (not paging_div) after leaving GTL screens (cherry picked from commit c047155) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1536062
Gaprindashvili backport details:
|
before: you still see paging there, and form buttons
after: you only see form buttons
Before the GTL rewrite, paging components used to be wrapped in
#paging_div
>#pc_div_1
> the toolbar.Thus any code that tries to hide
pc_div_1
used to hide the paging toolbar but keep#paging_div
(used for form buttons as well).This is now dead code, but looks like it's still needed when transitioning from a GTL screen (thus paging_div is there, and contains miq-pagination), to a form screen (thus paging_div doesn't get removed, but needs to only have form buttons).
Showing pc_div_1 is probably completely obsolete, because the GTL code does that by itself.
How removing paging is implemented - we simply call
setExtraClasses()
, used in other places (like clicking a toolbar) to remove the GTL classes. The complication is, this needs to work even without a running GTL instance - so wrapping the function inmiqGtlSetExtraClasses
to make it available without a gtl instance.@karelhala @martinpovolny please review
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1512957