cgkineo / adapt-pageNav

Navigation bar component (quicknav clone)
GNU General Public License v3.0
6 stars 4 forks source link

Allow hidden content objects to be used in navigation #68

Closed swashbuck closed 2 months ago

swashbuck commented 3 months ago

Subject of the issue

Currently, content objects that use "_isHidden": true behave similarly to topics that are locked in that they are grayed out and unclickable. The difference is that you can never navigate to a hidden content object, even when using its _id for the _customRouteId value.

In a use case that came up today, the course has a start page that's hidden from the menu. However, the client or designer (not sure which) wanted to be able to navigate to this page via the Previous button. The _customRouteId value for the Previous button is set to the start page's _id, but the button always appears disabled.

I understand excluding "_isAvailable": false content objects from Page Nav, as they aren't meant to be viewed at all. _isHidden just controls the content object's appearance on menus, though.

oliverfoster commented 3 months ago

_isHidden is not _isHiddenOnMenus

pageNav should naturally remove these content objects from normal next/back.

pageNav should behave as expected if a button is manually set on _customRouteId when routong to a hidden contentobject, i.e. not be greyed out unless locked

swashbuck commented 3 months ago

pageNav should behave as expect if a button is manually set on _customRouteId to route to a hidden contentobject, i.e. not be greyed out unless locked, etc.

@oliverfoster Ok, so you're saying the functionality of _customRouteId should be amended to allow for content objects that use _isHidden: true to be navigated to? No other changes to the Page Nav functionality?

oliverfoster commented 3 months ago

This is code in pageNav that looks at _isHidden. It forces the button to be visually disabled and behaviourally disabled and the tooltip to be hidden. I think both of those things should be removed and that _isHidden should be handled differently to make it more consistent with its intended behaviour.

The _customRouteId value should definitively override the model of the relevant button if defined.

Instead of: https://github.com/cgkineo/adapt-pageNav/blob/330a72b5bdeb2a843efa1e98e60aed169c07fd9f/js/PageNavModel.js#L44-L51 Do this:

      // Get models, skipping any undefined types (ex. deprecated button types)
      // Find buttonModel from config._customRouteId if not found in defined type
      let buttonModel = buttonConfig._customRouteId
        ? data.findById(buttonConfig._customRouteId)
        : buttonTypeModels[type];

      if (!buttonModel) continue;

And getPrevPage and getNextPage should exclude _isHidden pages as they do !_isAvailable pages:

Instead of these: https://github.com/cgkineo/adapt-pageNav/blob/330a72b5bdeb2a843efa1e98e60aed169c07fd9f/js/PageNavModel.js#L109-L110 https://github.com/cgkineo/adapt-pageNav/blob/330a72b5bdeb2a843efa1e98e60aed169c07fd9f/js/PageNavModel.js#L130-L131

Do this:

      const isNotShown = !page.get('_isAvailable') || page.get('_isHidden');
      if (isNotShown) continue;

That way, the next and prev buttons will not refer to _isHidden pages unless otherwise specified by _customRouteId and _isHidden will otherwise not change the behaviour of the buttons.

github-actions[bot] commented 2 months ago

:tada: This issue has been resolved in version 3.1.5 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: