dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Make the default tab for browsing communities and collections configurable in DSpace 8

Open abelgomez opened this issue 1 year ago • 2 comments

References

Add references/links to any related issues or PRs:

  • This PR is related to this discussion: https://groups.google.com/g/dspace-tech/c/IU3udR1bZfI/m/unOcszYACwAJ

Description

This PR adds one configuration option for both communities and collections to select a tab (other than the search tab) when it is navigated. This allows a behaviour similar to the one in DSpace 7. When the default tab is not the "search" tab, the "search" tab is moved at the end of the tabs ribbon for aesthetics purposes.

Instructions for Reviewers

After incorporating the changes, simply add the following options to your front-end configuration:

# Community Page Config
community:
  # Default tab to be shown when browsing a Community. Valid values are: comcols, search, or browse_<field>
  # <field> must be any of the configured "browse by" fields, e.g., dateissued, author, title, or subject
  # When the default tab is not the 'search' tab, the search tab is moved to the last position
  defaultBrowseTab: search

# Collection Page Config
collection:
  # Default tab to be shown when browsing a Collection. Valid values are: search, or browse_<field>
  # <field> must be any of the configured "browse by" fields, e.g., dateissued, author, title, or subject
  # When the default tab is not the 'search' tab, the search tab is moved to the last position
  defaultBrowseTab: search

List of changes in this PR:

  • The default routes for communities and collections have been modified. Now all tabs in a community/collection have a specific suffix to identify them (e.g.: https://sandbox.dspace.org/communities/8b632938-77c2-487c-81f0-e804f63e68e6/search; https://sandbox.dspace.org/communities/8b632938-77c2-487c-81f0-e804f63e68e6/comcols; etc.). See collection-page-routes.ts and community-page-routes.ts.
  • When a community/collection is accessed, the default tab is checked, and if no tab has been specified (by checking its URL), the route is changed to the default one. See comcol-page-browse-by.component.ts.
  • All other changes are minor: translations and interfaces holding configuration keys.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • [x] 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.
  • [x] My PR passes ESLint validation using yarn lint
  • [x] My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • [ ] My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • [x] My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • [x] My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • [ ] If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • [x] If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • [ ] If my PR fixes an issue ticket, I've linked them together.

abelgomez avatar Jul 03 '24 21:07 abelgomez

Ops! I just noticed I'm not passing the DSObject to the search component, and thus the results in the search tab are incorrect...

Please leave this PR on hold, and I'll try to commit an updated version asap.

abelgomez avatar Jul 04 '24 03:07 abelgomez

Now the search tab should work fine :)

abelgomez avatar Jul 04 '24 03:07 abelgomez

Hi @abelgomez, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!

github-actions[bot] avatar Jan 09 '25 22:01 github-actions[bot]

@abelgomez ; Just a friendly reminder that, if you want this PR to be considered for the 9.0 release, we will need to have the feedback above resolved soon (along with resolving any merge conflicts). Our 9.0 feature merger deadline is March 28. Anything not approved & merged by that date will unfortunately need to be delayed for the next major release.

tdonohue avatar Mar 12 '25 14:03 tdonohue

Hi @tdonohue,

I have checked the implementation, and while the default route I removed was not needed in most of the cases, you were right and there was a corner case I didn't think of.

I didn't notice that a default route was needed when no page reload is done (e.g., when navigating to the same community and collection that is being displayed using the breadcrumbs). In all other cases, the navigation was working since I was not removing the default path for the Collection ([dspace.ui.url]/collections/[uuid]) page, but only to the tab that is "directly shown" when the default tab is navigated (i.e., I was changing the path for ComcolSearchSectionComponent, not for ThemedCollectionPageComponent).

I also noticed that the code for selecting the default tab was a bit buggy, so I have reworked it.

As I mentioned, the default path that is being checked when directly navigating a Collection is still maintained in lines:

https://github.com/DSpace/dspace-angular/blob/cdcd886a3a371857dcf8fff7d2db32072647ec77/src/app/collection-page/collection-page-routes.ts#L83-L87

Thus, when a community/collection page is directly navigated, the base ComcolPageBrowseByComponent is automatically loaded since it is transitively contained by ThemedCollectionPageComponent. When ComcolPageBrowseByComponent is loaded, it checks in its ngOnInit method whether a trailing path is given in the url (search, subcoms-cols, etc.) or not.

If no trailing path is given, it checks which is the default tab in the configuration and directly navigates to it. On the contrary, if a trailing path is given, then the navigation and the tab that is shown by ComcolPageBrowseByComponent is handled by the "children" routes set in [src/app/collection-page/collection-page-routes.ts] as it used to.

Nevertheless, as I mentioned, and as per your suggestion, I have restored the default paths for ComcolPageBrowseByComponent, which as you pointed out may be needed in some corner cases. In such a case, the mechanism to navigate to "the right tab" works as explained above.

With these changes, everything should be working fine. The initial description of the PR still applies. The navigation should work similarly to DSpace 8, but with the exception that navigations to [dspace.ui.url]/collections/[uuid] will be redirected to [dspace.ui.url]/collections/[uuid]/search, which is the default "landing tab".

To test that other tabs can be used as the default "landing tab", you can set the following configuration properties:

community:
  defaultBrowseTab: comcols

collection:
  defaultBrowseTab: browse_dateissued

abelgomez avatar Mar 13 '25 23:03 abelgomez

Hi @artlowel ,

let me reply to this comment:

In my opinion it would be better to either: always put search at the back

This would make the default "landing" tab the last with the default configuration in DSpace 9, which most of the users would not change. I think that putting search at the end for most users is weird, and would change the look and feel w.r.t. DSpace 8.

always put whichever is the default tab at the front

I also thought about moving the default tab at the front, but that produced some "ugly" results.

E.g., if you select "By Issue Date" as the default in collections (as items were shown in DSpace 7, as far as I remember), the tabs will be shown as: "By Issue date", "Search", "By Author", "By Title", "By Subject", and "By Subject Category". That looks odd.

In the case of collections, I think that the most (non-default) typical use case would be to browse by "Communities and collections" (as it worked in DSpace 7).

So basically, the rationale was to avoid mixing "types of tabs" (i.e., treat the "browse by" tabs as a whole, and keep them always together), and keep a "familiar" look and feel (e.g., as it worked in DSpace 7).

There would be a third option, and that is not changing the tabs order at all, and simply select whichever is the "default" one in the position it has. But I think that, if users want to use a tab other than "Search" as the default, it's because they think it is a "secondary" feature on this page (and as such, it makes sense to move it to a less important place).

As a final thought: I also tried moving the default tab to the first position, but without "breaking" the "browse by" group (e.g., if "By author" is selected for Communities, the tabs would be "By Author", "By Issue Date", "By Title", "By Subject, "By Subject Category", "Search", "Subcommunities and Collections"). But I thought that would make the ordering a bit obscure to users and harder to implement. That's why I thought moving "Search" was the easiest one (and is a kind of solution of compromise)

In any case, if moving the "Search" tab is considered bad, I vote for not reordering the tabs at all.

Let me know if considering this, you think I should change something (or make something configurable).

abelgomez avatar Mar 21 '25 13:03 abelgomez

Hi @tdonohue ,

I have added minimal documentation to the wiki:

https://wiki.lyrasis.org/display/DSDOC9x/User+Interface+Configuration#UserInterfaceConfiguration-CommunityPageSettings https://wiki.lyrasis.org/display/DSDOC9x/User+Interface+Configuration#UserInterfaceConfiguration-CollectionPageSettings

Let me know if that's enough.

Cheers!

abelgomez avatar Mar 31 '25 21:03 abelgomez

Thanks @abelgomez ! That documentation looks good to me.

tdonohue avatar Mar 31 '25 21:03 tdonohue