VuFind
  1. VuFind
  2. VUFIND-777

Create a default tab per record driver option

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2
    • Component/s: None
    • Labels:
      None

      Description

      The ability to set a default tab per record driver would allow users to create a better user experience for vufind instances with mixed record types.

      It could be achieved my modifying the config.module.php array to include a default key

      'recorddriver_tabs' => array(
      'VuFind\RecordDriver\SolrDefault' =>
                      'tabs' => array(
                      'Holdings' => 'HoldingsILS', 'Description' => 'Description',
                      'TOC' => 'TOC', 'UserComments' => 'UserComments',
                      'Reviews' => 'Reviews', 'Excerpt' => 'Excerpt',
                      'HierarchyTree' => 'HierarchyTree', 'Map' => 'Map',
                      'Details' => 'StaffViewArray'
                      ),
                     'default' => 'Holdings'
                  )
      )

      A getDefaultTab method would need to be added to the AbstractRecord controller and the getAllTabs function would need to be modified to take account of the new key structure. The defaultTab variable in RecordController could then be set by calling the getDefaultTab function, falling back on the config.ini setting if no default key was set in module.config.php
      1. tabs.patch
        5 kB
        Luke O'Sullivan
      2. tabsDefault.patch
        16 kB
        Luke O'Sullivan
      3. tabsDefaultNew.patch
        13 kB
        Luke O'Sullivan

        Activity

        Hide
        Demian Katz added a comment -
        I like this idea. Having an associative array inside the tab configuration also offers the possibility of adding new features in the future (like inheriting settings from another driver, for example). Do you want to take a stab at implementing this?

        It might be wise to do this as two patches -- first patch will simply move the list of tabs to the 'tabs' sub-array; second patch can introduce the 'default' setting. We're going to have to think about how the new default tab should interact with the existing config.ini default tab setting; probably the smartest thing to do is to deprecate the config.ini setting... but that has knock-on effects of making upgrade scripts more complicated, etc. Might not be able to happen until 2.1. Thus, if we can at least commit the first part in 2.0 so that the first official release has the 'tabs' array in the right place, subsequent improvements in 2.1 will be that much easier.
        Show
        Demian Katz added a comment - I like this idea. Having an associative array inside the tab configuration also offers the possibility of adding new features in the future (like inheriting settings from another driver, for example). Do you want to take a stab at implementing this? It might be wise to do this as two patches -- first patch will simply move the list of tabs to the 'tabs' sub-array; second patch can introduce the 'default' setting. We're going to have to think about how the new default tab should interact with the existing config.ini default tab setting; probably the smartest thing to do is to deprecate the config.ini setting... but that has knock-on effects of making upgrade scripts more complicated, etc. Might not be able to happen until 2.1. Thus, if we can at least commit the first part in 2.0 so that the first official release has the 'tabs' array in the right place, subsequent improvements in 2.1 will be that much easier.
        Hide
        Luke O'Sullivan added a comment -
        Sure - I'll have a crack at it

        What is the VuFind2 position on getting users to modify the module.config.php array? Is it something you'd encourage (perhaps in a local module)? I'm just thinking how users could modify the default tab if they so wished. What about adding a recordtab.ini config file which contained the option to override the defaulttab setting for each recorddriver? I'm quite happy with the module.config.php approach - just trying to consider other possibilities...
        Show
        Luke O'Sullivan added a comment - Sure - I'll have a crack at it What is the VuFind2 position on getting users to modify the module.config.php array? Is it something you'd encourage (perhaps in a local module)? I'm just thinking how users could modify the default tab if they so wished. What about adding a recordtab.ini config file which contained the option to override the defaulttab setting for each recorddriver? I'm quite happy with the module.config.php approach - just trying to consider other possibilities...
        Hide
        Demian Katz added a comment -
        Yes, modifying the module.config.php array by overriding parts of it in a local module is encouraged. In a future version of VuFind, we may actually start to move away from .ini files in favor of more PHP configuration -- though for the moment we're sticking with the 1.x style .inis for ease of migration. (That's not a sure thing, of course -- .ini has some significant advantages, like the ability to be easily read in other languages like Java -- but at least some settings may be better served by PHP configuration).

        Regarding overriding module.config.php in a local module, it may be interesting to experiment with this a little to see exactly how the ZF2 config merging behavior interacts with the layout of the configuration; we should make sure there is a valid way to disable things as well as to add things -- it may be necessary to null out certain keys to get rid of them, and that may require the code to be prepared to see and skip null values in the array.
        Show
        Demian Katz added a comment - Yes, modifying the module.config.php array by overriding parts of it in a local module is encouraged. In a future version of VuFind, we may actually start to move away from .ini files in favor of more PHP configuration -- though for the moment we're sticking with the 1.x style .inis for ease of migration. (That's not a sure thing, of course -- .ini has some significant advantages, like the ability to be easily read in other languages like Java -- but at least some settings may be better served by PHP configuration). Regarding overriding module.config.php in a local module, it may be interesting to experiment with this a little to see exactly how the ZF2 config merging behavior interacts with the layout of the configuration; we should make sure there is a valid way to disable things as well as to add things -- it may be necessary to null out certain keys to get rid of them, and that may require the code to be prepared to see and skip null values in the array.
        Hide
        Luke O'Sullivan added a comment -
        Two patches

        tabs.patch - Alters the recorddriver_tab section of module.config.php and adjusts the plugin manager accordingly

        tabsDefault.patch eliminates the use of default tab settings in config.ini and moves default settings for all recorddrivers to module.config.php
        Show
        Luke O'Sullivan added a comment - Two patches tabs.patch - Alters the recorddriver_tab section of module.config.php and adjusts the plugin manager accordingly tabsDefault.patch eliminates the use of default tab settings in config.ini and moves default settings for all recorddrivers to module.config.php
        Hide
        Demian Katz added a comment - - edited
        I've applied tabs.patch here:

        https://github.com/vufind-org/vufind/commit/89719020d1aa7fa81826f6f5d4b3f288e86cc00b

        I made some very small adjustments; as a result, tabsDefault.patch will no longer cleanly apply. Do you mind merging the latest master with your branch and regenerating tabsDefault.patch?
        Show
        Demian Katz added a comment - - edited I've applied tabs.patch here: https://github.com/vufind-org/vufind/commit/89719020d1aa7fa81826f6f5d4b3f288e86cc00b I made some very small adjustments; as a result, tabsDefault.patch will no longer cleanly apply. Do you mind merging the latest master with your branch and regenerating tabsDefault.patch?
        Hide
        Luke O'Sullivan added a comment -
        New Default Tabs Patch - I had a few problems with the merge - Hopefully this patch resolves all those
        Show
        Luke O'Sullivan added a comment - New Default Tabs Patch - I had a few problems with the merge - Hopefully this patch resolves all those
        Hide
        Demian Katz added a comment -
        I've reimplemented this in a slightly more backward-compatible way -- you can now set defaultTab to null in module.config.php in order to fail over to the old .ini-driven behavior. I've also created a separate section on module.config.php for collection tabs rather than using a fake class name within the main tab configuration array -- seems a little cleaner/simpler.
        Show
        Demian Katz added a comment - I've reimplemented this in a slightly more backward-compatible way -- you can now set defaultTab to null in module.config.php in order to fail over to the old .ini-driven behavior. I've also created a separate section on module.config.php for collection tabs rather than using a fake class name within the main tab configuration array -- seems a little cleaner/simpler.

          People

          • Assignee:
            Demian Katz
            Reporter:
            Luke O'Sullivan
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: