pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
304 stars 444 forks source link

FieldOptions with isOrderable doesn't maintain order #10529

Open NateWr opened 14 hours ago

NateWr commented 14 hours ago

Describe the bug When a <FieldOptions> component has the isOrderable prop, the options can be reordered. However, when loading the field, the options are not displayed in the order they were last saved in.

The only place an orderable FieldOptions is used is Settings > Website > Appearance > Setup > Sidebar. There is a workaround on the backend to reorder the options before they are sent to the Vue component.

https://github.com/pkp/pkp-lib/blob/b8d985fb8baebe981935ba2175616c6fd1a923b8/classes/components/forms/context/PKPAppearanceSetupForm.php#L53-L71

To Reproduce The easiest way to reproduce this is to remove the workaround code in PKPAppearanceSetupForm.php and then reproduce it with the sidebar setting:

  1. Apply the following diff to main:
diff --git a/classes/components/forms/context/PKPAppearanceSetupForm.php b/classes/components/forms/context/PKPAppearanceSetupForm.php
index 84c05c16c7..684d037c1c 100644
--- a/classes/components/forms/context/PKPAppearanceSetupForm.php
+++ b/classes/components/forms/context/PKPAppearanceSetupForm.php
@@ -43,33 +43,18 @@ class PKPAppearanceSetupForm extends FormComponent
         $this->action = $action;
         $this->locales = $locales;
         $sidebarOptions = [];
-        $enabledOptions = [];
-        $disabledOptions = [];

         $currentBlocks = (array) $context->getData('sidebar');

         $plugins = PluginRegistry::loadCategory('blocks', true);

-        foreach ($currentBlocks as $plugin) {
-            if (isset($plugins[$plugin])) {
-                $enabledOptions[] = [
-                    'value' => $plugin,
-                    'label' => htmlspecialchars($plugins[$plugin]->getDisplayName()),
-                ];
-            }
-        }
-
         foreach ($plugins as $pluginName => $plugin) {
-            if (!in_array($pluginName, $currentBlocks)) {
-                $disabledOptions[] = [
-                    'value' => $pluginName,
-                    'label' => htmlspecialchars($plugin->getDisplayName()),
-                ];
-            }
+            $sidebarOptions[] = [
+                'value' => $pluginName,
+                'label' => htmlspecialchars($plugin->getDisplayName()),
+            ];
         }

-        $sidebarOptions = array_merge($enabledOptions, $disabledOptions);
-
         $this->addField(new FieldUploadImage('pageHeaderLogoImage', [
             'label' => __('manager.setup.logo'),
             'value' => $context->getData('pageHeaderLogoImage'),
  1. Go to Settings > Website > Appearance > Setup > Sidebar.
  2. Enable one of the sidebar blocks near the bottom of the list and move it to the top.
  3. Save the form.
  4. Reload the page.
  5. See the enabled sidebar block is at the bottom again.

What application are you using? This effects as far back as stable-3_3_0 and probably earlier.

Additional information This bug is causing problems for me when using the field as a theme option. I tried using a similar backend workaround, but it causes serious side effects. In short, I must fetch the value of the theme option to sort the order of the options. However, once I fetch the value of the theme option, all theme options are cached. This means the resorting has to be done after all theme options are defined. I can do this, but it would break any theme options defined in a child theme.

NateWr commented 14 hours ago

The following PR fixes the issue in FieldOptions.vue by sorting the options in the mounted hook. It also removes the workaround code from PKPAppearanceSetupForm.php.

I opened the PR against main because I'm having trouble running npm run [build | start] in my stable-3_3_0 checkout. I believe it may be a node version issue. Ideally, this would be back-ported to stable-3_3_0.

PRs: https://github.com/pkp/ui-library/pull/429 (main) https://github.com/pkp/pkp-lib/pull/10530 (main)

TESTS ONLY: https://github.com/pkp/ojs/pull/4475