PF4 pagination

Hello,
Yesterday on the UX Interest Group Meeting 13/12/21 we talked about Fixes #34133 - Use PF4 pagination as default.
In the PR, the new PF4 pagination is added and the initial thought was to keep the PF3 pagination and its helpers as they are also consumed by plugins.

after a small discussion with the group, a few thoughts were raised:

  • Since PF3 is becoming a technical debt that will eventually prevent us from upgrading React, we should migrate to PF4.

  • We should drop the PF3 pagination in the PR and its helpers which should be used only in the Pagination component.

  • Stop using the usePaginationOptions hook across plugins and instead just import the PF4 pagination component from Foreman which already provides those options.

  • Enable plugins to override all of the pagination props for full control.

  • Finally, we need to open multiple PRs across plugins in order to make the change smooth.

Would be glad to get your feedback on it, and if you’re a plugin maintainer please comment if you can help with a PR on the plugin.

PR: Fixes #34133 - Use PF4 pagination as default
Cheers!

3 Likes

I’ll can take care of Leapp plugin & eventually help with the REX

1 Like

I didn’t check all plugins, but from what I saw the main usage of the hook was to convert the options to pf4 format for use in the pf4 pagination component directly.
To reduce breakages, plugins can already be modified to use the foreman core pagination component without the hook. Until the core change is merged that means they will get the pf3 pagination for a few days, but once we merge the core side everywhere we use pagination in the application will use the pf4 implementation.

interesting approach there,
what I am thinking of is to merge the current PR: https://github.com/theforeman/foreman/pull/8990 where both PF3 and PF4 exist,
so plugins could already start switching to use the Foreman core pagination,
and then in a follow up that I will open, removing the old PF3 components.

Imho in this approach some plugins won’t break and we give them a good alternative to their own PF4 implementations,
and also we will have smaller PRs for adding and removing the pagination components.

I’m afraid that might lead to no changes in the plugins and we’ll be stuck with both implementations, until we decide to drop the pf3 one and break/update the plugins then. I’ve just had to open many prs to plugins to clean up code that was deprecated months ago when cleaning up deprecations.

in an ideal world…
OK, let’s just do it then :slight_smile:

I updated the core PR: https://github.com/theforeman/foreman/pull/8990
which drops ~500 lines of code,
and after reviews, I will start opening PRs to plugins.

1 Like

There are currently open PRs to plugins, see the tracker in Tracker #34185: use PF4 pagination - Foreman
the core PR is ready to be merged and removes the PF3 implementation, another Pagination wrapper and pagination-options hooks as they moved into the PF4 component implementation: Fixes #34133 - Use PF4 pagination as default by Ron-Lavi · Pull Request #8990 · theforeman/foreman · GitHub

we kept the files and the export points of the old code just so plugins will succeed to build,
though the inner code was removed and replaced with a deprecation warning, so the pagination itself can break in plugins.

Thanks @ezr-ondrej for pushing the button yesterday :tada:
so this is the current status:

there is a tracker in Tracker #34185: use PF4 pagination - Foreman
we still need to open PRs for those plugins, can someone help? opened Redmine issues:

1 Like

I’ll take care of remote_execution (Fixes #34201 - Use new Foreman <Pagination> by stejskalleos · Pull Request #682 · theforeman/foreman_remote_execution · GitHub) and Leapp (in progress)

1 Like

And Leapp: Fixes #34276 - consume PF4 pagination from core by stejskalleos · Pull Request #110 · theforeman/foreman_leapp · GitHub