[CST-5449] "Browse by" pages are missing "Now showing" contextual information

[davide-negretti]

References

• Fixes [Usability issue] "Browse by" pages are missing "Now showing" contextual information #369

Description

"Now showing" has been added to "browse by" pages also when showPaginator is false

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes TSLint validation using yarn run lint
• My PR doesn't introduce circular dependencies
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

[MarieVerdonck]

You have to merge in the latest community main to render this branch (otherwise error:

zone-evergreen.js:659 Unhandled Promise rejection: Cannot read properties of undefined (reading 'nameSpace') ; Zone: <root> ; Task: Promise.then ; Value: TypeError: Cannot read properties of undefined (reading 'nameSpace')
    at Object.getBase [as useFactory] (app.module.ts:68:23)

The results of a page are present twice, with the second being in a narrower div, centered on page, causing an indent. Also the previous/next buttons then are left/right aligned within this indented div of duplicated page of results (see screenshot)

Considering the complaints from the original issue boil down to:

• No "Now showing items __ of ___"
• Page of pagination not clear like on regular pagination buttons (<<|1|2|3|...|{lastPagNr}|>>)

I would undo these changes and just set enableArrows to false here.

Browse already has two optional views built in: the one currently on demo (enabledArrowse=true), and the regular pagination one (enableArrows=false), so with "Now showing items __ of ___" and pagination buttons. The latter of which uses shared ds-viewable-collection, the former is custom to the browse pages.

The 'enableArrows=true' view was likely added since this is more in line with the old view on DS6 (example), but the pagination arrows include more info on pagination and allow you to skip multiple pages at once, so seems much more user friendly which would be an improvement.

This is the view just on main with enabledArrows=false change:

In regards with compatibility with the suggested changes in #1561 if we go for just the enableArrows=false change, then the change there like showing tooltip on disabled next button would still make the 'enableArrows=true' view slighly more user friendly, but since this view wouldn't be default, it can be removed there if requested.

[MarieVerdonck]

So I would just set enableArrows=false here, and undo other changes

[MarieVerdonck]

You could keep these changes in ds-viewable-collection (provided the duplicate page with indent rendering is fixed) and enable the alternate arrow view with enableArrows boolean there.
But the 1|2|3 pagination buttons are more user friendly, so by default I would use those.
This would make it so the 'enableArrows=true' view also has the 'Showing x of x' text, which is present in DS6, which would improve that view as well.

[atarix83]

@tdonohue @MarieVerdonck could you check again, the issue should be resolved now.

I left the enableArrows to true so you can test the behaviour with next and prev button, in the #1561 can be turned to false

[tdonohue]

👍 Thanks @davidenegretti-4science and @atarix83 ! Tested this today and it works as described. All Browse by pages have "Now Showing..." at the top.

Per @MarieVerdonck 's feedback about using the 1|2|3 pagination instead of the Previous/Next buttons: As discussed with @artlowel in yesterday's meeting, this PR can go forward with the Previous/Next button approach. As we mentioned though, @MarieVerdonck can always switch it over to using 1|2|3 pagination in PR #1561 -- as I do agree that, if it's easy to achieve, the 1|2|3 pagination may provide us with a better user experience.

All in all, I think this PR looks good enough to move forward. @MarieVerdonck: Did you have any final comments to add? I believe all your feedback/bugs have been resolved (other than the 1|2|3 pagination, obviously). But, I'll give you a chance to give this one last look.

[MarieVerdonck]

Previous issue seems fixed and verified you can now go to the 123 view again, now with the [showPaginator]="true" of ds-viewable-collection in browse-by.component.html. Feel free to merge so we can rebase in #1561 so we can see how these changes interact with those (location go back button / tooltip on last next button might not work any more/need changing). Afterwards we can enable the 123 view by default if everyone agrees this provides the better UX concerning pagination.

[MarieVerdonck]

Setting the showPaginator to true here seems to enable the 123 view in browse

[tdonohue]

@MarieVerdonck : Yes, if we find the 1|2|3 pagination is easy to enable & works well, I'd be in favor of doing that in your separate #1561 PR.

[tdonohue]

Merging as this is at +2. Thanks @davide-negretti, and @pratik-peerbits and @atarix83 !